| From 045afc24124d80c6998d9c770844c67912083506 Mon Sep 17 00:00:00 2001 |
| From: Will Deacon <will.deacon@arm.com> |
| Date: Mon, 8 Apr 2019 12:45:09 +0100 |
| Subject: arm64: futex: Fix FUTEX_WAKE_OP atomic ops with non-zero result value |
| |
| From: Will Deacon <will.deacon@arm.com> |
| |
| commit 045afc24124d80c6998d9c770844c67912083506 upstream. |
| |
| Rather embarrassingly, our futex() FUTEX_WAKE_OP implementation doesn't |
| explicitly set the return value on the non-faulting path and instead |
| leaves it holding the result of the underlying atomic operation. This |
| means that any FUTEX_WAKE_OP atomic operation which computes a non-zero |
| value will be reported as having failed. Regrettably, I wrote the buggy |
| code back in 2011 and it was upstreamed as part of the initial arm64 |
| support in 2012. |
| |
| The reasons we appear to get away with this are: |
| |
| 1. FUTEX_WAKE_OP is rarely used and therefore doesn't appear to get |
| exercised by futex() test applications |
| |
| 2. If the result of the atomic operation is zero, the system call |
| behaves correctly |
| |
| 3. Prior to version 2.25, the only operation used by GLIBC set the |
| futex to zero, and therefore worked as expected. From 2.25 onwards, |
| FUTEX_WAKE_OP is not used by GLIBC at all. |
| |
| Fix the implementation by ensuring that the return value is either 0 |
| to indicate that the atomic operation completed successfully, or -EFAULT |
| if we encountered a fault when accessing the user mapping. |
| |
| Cc: <stable@kernel.org> |
| Fixes: 6170a97460db ("arm64: Atomic operations") |
| Signed-off-by: Will Deacon <will.deacon@arm.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| arch/arm64/include/asm/futex.h | 16 ++++++++-------- |
| 1 file changed, 8 insertions(+), 8 deletions(-) |
| |
| --- a/arch/arm64/include/asm/futex.h |
| +++ b/arch/arm64/include/asm/futex.h |
| @@ -30,8 +30,8 @@ do { \ |
| " prfm pstl1strm, %2\n" \ |
| "1: ldxr %w1, %2\n" \ |
| insn "\n" \ |
| -"2: stlxr %w3, %w0, %2\n" \ |
| -" cbnz %w3, 1b\n" \ |
| +"2: stlxr %w0, %w3, %2\n" \ |
| +" cbnz %w0, 1b\n" \ |
| " dmb ish\n" \ |
| "3:\n" \ |
| " .pushsection .fixup,\"ax\"\n" \ |
| @@ -50,30 +50,30 @@ do { \ |
| static inline int |
| arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *_uaddr) |
| { |
| - int oldval = 0, ret, tmp; |
| + int oldval, ret, tmp; |
| u32 __user *uaddr = __uaccess_mask_ptr(_uaddr); |
| |
| pagefault_disable(); |
| |
| switch (op) { |
| case FUTEX_OP_SET: |
| - __futex_atomic_op("mov %w0, %w4", |
| + __futex_atomic_op("mov %w3, %w4", |
| ret, oldval, uaddr, tmp, oparg); |
| break; |
| case FUTEX_OP_ADD: |
| - __futex_atomic_op("add %w0, %w1, %w4", |
| + __futex_atomic_op("add %w3, %w1, %w4", |
| ret, oldval, uaddr, tmp, oparg); |
| break; |
| case FUTEX_OP_OR: |
| - __futex_atomic_op("orr %w0, %w1, %w4", |
| + __futex_atomic_op("orr %w3, %w1, %w4", |
| ret, oldval, uaddr, tmp, oparg); |
| break; |
| case FUTEX_OP_ANDN: |
| - __futex_atomic_op("and %w0, %w1, %w4", |
| + __futex_atomic_op("and %w3, %w1, %w4", |
| ret, oldval, uaddr, tmp, ~oparg); |
| break; |
| case FUTEX_OP_XOR: |
| - __futex_atomic_op("eor %w0, %w1, %w4", |
| + __futex_atomic_op("eor %w3, %w1, %w4", |
| ret, oldval, uaddr, tmp, oparg); |
| break; |
| default: |