unbreak generic futex_atomic_cmpxchg_inatomic() on UP
Subsystems (2)
RFC PATCH 1/1 unbreak generic futex_atomic_cmpxchg_inatomic() on UP
[Resend. I messed up the To: line the first time]
As has been reported recently by Lennert Buytenhek, robust futexes are broken on ARM:
Quoted 5 lines
If you're also running into glibc's tst-robust1 test suite test locking up your ARM machine, you're probably running into the fact that
asm-arm/futex.hincludesasm-generic/futex.h, andasm-generic/futex.hdefinesfutex_atomic_cmpxchg_inatomic()to return-ENOSYS. This causeshandle_futex_death()to loop forever.
I can confirm this statement: building glibc-2.4 with NPTL on ARM hangs the kernel when the test suite reaches tst-robust1.
The problem is that kernel/futex.c expects futex_atomic_cmpxchg_inatomic() to return -EFAULT or the new value. It doesn't expect -ENOSYS at all, and generally -ENOSYS causes the futex code to loop, hanging the kernel.
The higher-end archs (x86, sparc64, ppc64, etc) provide fully-functional asm/futex.h implementations, but a number of archs (alpha, arm, arm26, avr32, blackfin, cris, h8300, m32r, m68k, mk68knommu, sh64, sparc, um, v850, and xtensa) use asm-generic/futex.h, which makes robust futexes horribly broken on them. There have also been reports recently that PI futexes are broken due to the generic futex_atomic_cmpxchg_inatomic() just being an -ENOSYS stub.
The patch below implements the generic futex_atomic_cmpxchg_inatomic() in terms of __copy_{from,to}_user_inatomic() and preempt_{disable,enable}(). It obviously doesn't support SMP, but UP-only support should go a long way for users of the affected archs.
I'm using this patch now and it has allowed me to build and use glibc-2.4 with NPTL on ARM (glibc-2.4-11.src.rpm from FC5 + ARM fixes). (Finally I can ditch LinuxThreads :->)
Comments?
/Mikael
include/asm-generic/futex.h renamed from include/asm-generic/futex.h.~1~ +28 -0
@@ -4,6 +4,7 @@#ifdef __KERNEL__#include <linux/futex.h> +#include <linux/preempt.h> #include <asm/errno.h> #include <asm/uaccess.h>
@@ -52,7 +53,34 @@ futex_atomic_op_inuser (int encoded_op,static inline int futex_atomic_cmpxchg_inatomic (int __user *uaddr,int oldval,int newval) { +#ifdef CONFIG_SMP return -ENOSYS ; +#else +int curval, ret; + +if (!access_ok (VERIFY_WRITE , uaddr,sizeof (int ))) +return -EFAULT ; + +preempt_disable (); + + ret = -EFAULT ; +if (__copy_from_user_inatomic (&curval, uaddr,sizeof (int ))) + goto out; + + ret = curval; +if (curval != oldval) + goto out; + + ret = -EFAULT ; +if (__copy_to_user_inatomic (uaddr, &newval,sizeof (int ))) + goto out; + + ret = newval; + + out: +preempt_enable (); +return ret; +#endif }#endif
include/asm-generic/futex.h futex_atomic_op_inuser (int encoded_op, · line 58
Quoted 7 lines · Mikael Pettersson
include/asm-generic/futex.h renamed from include/asm-generic/futex.h.~1~ +2
-0@@ -52,7 +53,34 @@ futex_atomic_op_inuser (int encoded_op,static inline int futex_atomic_cmpxchg_inatomic (int __user *uaddr,int oldval,int newval) { +#ifdef CONFIG_SMP return -ENOSYS ; +#else
Since the callers of futex_atomic_cmpxchg_inatomic() don't really seem prepared to deal with -ENOSYS (e.g. the handle_futex_death() infinite loop when it gets -ENOSYS), it seems better never to return -ENOSYS from this function at all.
What if you just stick an #error in here in the SMP case?
RFC PATCH 1/1 unbreak generic futex_atomic_cmpxchg_inatomic() on UP
In reply to Mikael Pettersson-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
What's the state of this patch? I can confirm tst-robust1 from glibc testsuite locks a armv5 machine hard. With this patch applied, the test succeeds.
Quoted 7 lines
The higher-end archs (x86, sparc64, ppc64, etc) provide fully-functional
asm/futex.himplementations, but a number of archs (alpha, arm, arm26, avr32, blackfin, cris, h8300, m32r, m68k, mk68knommu, sh64, sparc, um, v850, and xtensa) useasm-generic/futex.h, which makes robust futexes horribly broken on them. There have also been reports recently that PI futexes are broken due to the genericfutex_atomic_cmpxchg_inatomic()just being an-ENOSYSstub.
This an effective local DOS bug on the affected architectures, too..
Quoted 49 lines
The patch below implements the generic
futex_atomic_cmpxchg_inatomic()in terms of __copy_{from,to}_user_inatomic() and preempt_{disable,enable}(). It obviously doesn't support SMP, but UP-only support should go a long way for users of the affected archs.I'm using this patch now and it has allowed me to build and use glibc-2.4 with NPTL on ARM (glibc-2.4-11.src.rpm from FC5 + ARM fixes). (Finally I can ditch LinuxThreads :->)
Comments?
/Mikael
include/asm-generic/futex.h renamed from include/asm-generic/futex.h.~1~ +22
-0@@ -4,6 +4,7 @@#ifdef __KERNEL__#include <linux/futex.h> +#include <linux/preempt.h> #include <asm/errno.h> #include <asm/uaccess.h> @@ -52,7 +53,34 @@ futex_atomic_op_inuser (int encoded_op,static inline int futex_atomic_cmpxchg_inatomic (int __user *uaddr,int oldval,int newval) { +#ifdef CONFIG_SMP return -ENOSYS ; +#else +int curval, ret; + +if (!access_ok (VERIFY_WRITE , uaddr,sizeof (int ))) +return -EFAULT ; + +preempt_disable (); + + ret = -EFAULT ; +if (__copy_from_user_inatomic (&curval, uaddr,sizeof (int ))) + goto out; + + ret = curval; +if (curval != oldval) + goto out; + + ret = -EFAULT ; +if (__copy_to_user_inatomic (uaddr, &newval,sizeof (int ))) + goto out; +
include/asm-generic/futex.h futex_atomic_op_inuser (int encoded_op,
Quoted 10 lines
include/asm-generic/futex.h renamed from include/asm-generic/futex.h.~1~ +6
-1+ ret = newval; + + out: +preempt_enable (); +return ret; +#endif }#endif -
- --
"rm -rf" only sounds scary if you don't have backups -----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFHA+6aibPvMsrqrwMRAlMuAKCBf7qpD2dETgU+RgnDG4ArVvFp3gCgrewq 4KvOt40U1MAPM7g4F/Ps5jk=
=68gr
-----END PGP SIGNATURE-----