lib/38482: C compiler can generate non-restartable code within a RAS

[scw%netbsd.org@localhost]

>Number:         38482
>Category:       lib
>Synopsis:       C compiler can generate non-restartable code within a RAS.
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Apr 21 21:00:00 +0000 2008
>Originator:     Steve Woodford
>Release:        NetBSD 4.99.60
>Organization:
>Environment:
Architecture: arm, hppa, m68000, mips, sh3, sparc, vax
Machine: arm, hppa, m68000, mips, sh3, sparc, vax
>Description:
I've recently noticed occasional SEGVs with threaded programs on ARM.
This turns out to be caused by dereferencing a bad pointer value in the
code between the RAS_START and RAS_END in
common/lib/libc/atomic/atomic_init_testset.c:_atomic_cas_up().

Examining the assembly code produced by Gcc shows the reason:

 108:   e1a03000        mov     r3, r0
0000010c <_atomic_cas_ras_start>:
 10c:   e5900000        ldr     r0, [r0]
 110:   e1500001        cmp     r0, r1
 114:   11a0f00e        movne   pc, lr
 118:   e5832000        str     r2, [r3]
0000011c <_atomic_cas_ras_end>:
 11c:   e1a0f00e        mov     pc, lr

Register r0 is modified within the sequence, therefore a bogus pointer
dereference is almost inevitable if the sequence is restarted.

It is likely all other archs which lack native CAS are at risk from
similar lossage. A quick check shows arm, hppa, m68000, mips, sh3, sparc,
and vax all use atomic_init_testset.c. I've only examined the compiled
code on ARM so I can't verify the other archs are similarly affected.
Even if an arch isn't currently affected, any future toolchain update or
minor change to the source for _atomic_cas_up() could easily break it.

>How-To-Repeat:
Run threaded code on affected archs (ARM in my case).
Observe occasional, seemingly random, SEGVs.

>Fix:
Rather than fighting the compiler, I propose we implement _atomic_cas_up()
in asm code for each affected arch. I can do ARM and m68000.

In addition, the rasctl(2) manual page should be updated to strongly
recommend that RASs are implemented in asm code only. Perhaps we should go
so far as removing the RAS_START/RAS_END macros from the API, though that
may preclude using in-line asm...

An audit of the tree for similar code sequences would also be a good idea.
For example: pthread_lock.c.