| Message ID | 18915.54309.632699.346771@cargo.ozlabs.ibm.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 306a82881b14d950d59e0b59a55093a07d82aa9a |
| Delegated to: | Paul Mackerras |
| Headers | show |
Paul Mackerras wrote: > Can anyone see any reason why the FUTEX_OP_SET case can't just do a > __put_user? Because you need to return the old value. Despite the name, it's an exchange operation not a set. r~
I'd like to understand the implications of this bug. Obviously applications using the futex system can be affected, but does anybody know whether GNU software packages suffer from this problem. I mean glibc (nptl) uses futexes, so does gdb and gcc. will this bug hurt them ? Paul Mackerras wrote: > Richard Henderson pointed out that the powerpc __futex_atomic_op has a > bug: it will write the wrong value if the stwcx. fails and it has to > retry the lwarx/stwcx. loop, since 'oparg' will have been overwritten > by the result from the first time around the loop. This happens > because it uses the same register for 'oparg' (an input) as it uses > for the result. > > This fixes it by using separate registers for 'oparg' and 'ret'. > > Cc: stable@kernel.org > Signed-off-by: Paul Mackerras <paulus@samba.org> > --- > Can anyone see any reason why the FUTEX_OP_SET case can't just do a > __put_user? > > diff --git a/arch/powerpc/include/asm/futex.h b/arch/powerpc/include/asm/futex.h > index 6d406c5..9696cc3 100644 > --- a/arch/powerpc/include/asm/futex.h > +++ b/arch/powerpc/include/asm/futex.h > @@ -27,7 +27,7 @@ > PPC_LONG "1b,4b,2b,4b\n" \ > ".previous" \ > : "=&r" (oldval), "=&r" (ret) \ > - : "b" (uaddr), "i" (-EFAULT), "1" (oparg) \ > + : "b" (uaddr), "i" (-EFAULT), "r" (oparg) \ > : "cr0", "memory") > > static inline int futex_atomic_op_inuser (int encoded_op, int __user *uaddr) > @@ -47,19 +47,19 @@ static inline int futex_atomic_op_inuser (int encoded_op, int __user *uaddr) > > switch (op) { > case FUTEX_OP_SET: > - __futex_atomic_op("", ret, oldval, uaddr, oparg); > + __futex_atomic_op("mr %1,%4\n", ret, oldval, uaddr, oparg); > break; > case FUTEX_OP_ADD: > - __futex_atomic_op("add %1,%0,%1\n", ret, oldval, uaddr, oparg); > + __futex_atomic_op("add %1,%0,%4\n", ret, oldval, uaddr, oparg); > break; > case FUTEX_OP_OR: > - __futex_atomic_op("or %1,%0,%1\n", ret, oldval, uaddr, oparg); > + __futex_atomic_op("or %1,%0,%4\n", ret, oldval, uaddr, oparg); > break; > case FUTEX_OP_ANDN: > - __futex_atomic_op("andc %1,%0,%1\n", ret, oldval, uaddr, oparg); > + __futex_atomic_op("andc %1,%0,%4\n", ret, oldval, uaddr, oparg); > break; > case FUTEX_OP_XOR: > - __futex_atomic_op("xor %1,%0,%1\n", ret, oldval, uaddr, oparg); > + __futex_atomic_op("xor %1,%0,%4\n", ret, oldval, uaddr, oparg); > break; > default: > ret = -ENOSYS; > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-dev >
diff --git a/arch/powerpc/include/asm/futex.h b/arch/powerpc/include/asm/futex.h index 6d406c5..9696cc3 100644 --- a/arch/powerpc/include/asm/futex.h +++ b/arch/powerpc/include/asm/futex.h @@ -27,7 +27,7 @@ PPC_LONG "1b,4b,2b,4b\n" \ ".previous" \ : "=&r" (oldval), "=&r" (ret) \ - : "b" (uaddr), "i" (-EFAULT), "1" (oparg) \ + : "b" (uaddr), "i" (-EFAULT), "r" (oparg) \ : "cr0", "memory") static inline int futex_atomic_op_inuser (int encoded_op, int __user *uaddr) @@ -47,19 +47,19 @@ static inline int futex_atomic_op_inuser (int encoded_op, int __user *uaddr) switch (op) { case FUTEX_OP_SET: - __futex_atomic_op("", ret, oldval, uaddr, oparg); + __futex_atomic_op("mr %1,%4\n", ret, oldval, uaddr, oparg); break; case FUTEX_OP_ADD: - __futex_atomic_op("add %1,%0,%1\n", ret, oldval, uaddr, oparg); + __futex_atomic_op("add %1,%0,%4\n", ret, oldval, uaddr, oparg); break; case FUTEX_OP_OR: - __futex_atomic_op("or %1,%0,%1\n", ret, oldval, uaddr, oparg); + __futex_atomic_op("or %1,%0,%4\n", ret, oldval, uaddr, oparg); break; case FUTEX_OP_ANDN: - __futex_atomic_op("andc %1,%0,%1\n", ret, oldval, uaddr, oparg); + __futex_atomic_op("andc %1,%0,%4\n", ret, oldval, uaddr, oparg); break; case FUTEX_OP_XOR: - __futex_atomic_op("xor %1,%0,%1\n", ret, oldval, uaddr, oparg); + __futex_atomic_op("xor %1,%0,%4\n", ret, oldval, uaddr, oparg); break; default: ret = -ENOSYS;
Richard Henderson pointed out that the powerpc __futex_atomic_op has a bug: it will write the wrong value if the stwcx. fails and it has to retry the lwarx/stwcx. loop, since 'oparg' will have been overwritten by the result from the first time around the loop. This happens because it uses the same register for 'oparg' (an input) as it uses for the result. This fixes it by using separate registers for 'oparg' and 'ret'. Cc: stable@kernel.org Signed-off-by: Paul Mackerras <paulus@samba.org> --- Can anyone see any reason why the FUTEX_OP_SET case can't just do a __put_user?