| From 8112703f5ecc572a5632333fc3640a88eaff144b Mon Sep 17 00:00:00 2001 |
| From: Jiri Slaby <jslaby@suse.cz> |
| Date: Mon, 23 Oct 2017 13:41:51 +0200 |
| Subject: futex: futex_wake_op, do not fail on invalid op |
| |
| [ Upstream commit e78c38f6bdd900b2ad9ac9df8eff58b745dc5b3c ] |
| |
| In commit 30d6e0a4190d ("futex: Remove duplicated code and fix undefined |
| behaviour"), I let FUTEX_WAKE_OP to fail on invalid op. Namely when op |
| should be considered as shift and the shift is out of range (< 0 or > 31). |
| |
| But strace's test suite does this madness: |
| |
| futex(0x7fabd78bcffc, 0x5, 0xfacefeed, 0xb, 0x7fabd78bcffc, 0xa0caffee); |
| futex(0x7fabd78bcffc, 0x5, 0xfacefeed, 0xb, 0x7fabd78bcffc, 0xbadfaced); |
| futex(0x7fabd78bcffc, 0x5, 0xfacefeed, 0xb, 0x7fabd78bcffc, 0xffffffff); |
| |
| When I pick the first 0xa0caffee, it decodes as: |
| |
| 0x80000000 & 0xa0caffee: oparg is shift |
| 0x70000000 & 0xa0caffee: op is FUTEX_OP_OR |
| 0x0f000000 & 0xa0caffee: cmp is FUTEX_OP_CMP_EQ |
| 0x00fff000 & 0xa0caffee: oparg is sign-extended 0xcaf = -849 |
| 0x00000fff & 0xa0caffee: cmparg is sign-extended 0xfee = -18 |
| |
| That means the op tries to do this: |
| |
| (futex |= (1 << (-849))) == -18 |
| |
| which is completely bogus. The new check of op in the code is: |
| |
| if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) { |
| if (oparg < 0 || oparg > 31) |
| return -EINVAL; |
| oparg = 1 << oparg; |
| } |
| |
| which results obviously in the "Invalid argument" errno: |
| |
| FAIL: futex |
| =========== |
| |
| futex(0x7fabd78bcffc, 0x5, 0xfacefeed, 0xb, 0x7fabd78bcffc, 0xa0caffee) = -1: Invalid argument |
| futex.test: failed test: ../futex failed with code 1 |
| |
| So let us soften the failure to print only a (ratelimited) message, crop |
| the value and continue as if it were right. When userspace keeps up, we |
| can switch this to return -EINVAL again. |
| |
| [v2] Do not return 0 immediatelly, proceed with the cropped value. |
| |
| Fixes: 30d6e0a4190d ("futex: Remove duplicated code and fix undefined behaviour") |
| Signed-off-by: Jiri Slaby <jslaby@suse.cz> |
| Cc: Ingo Molnar <mingo@redhat.com> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Darren Hart <dvhart@infradead.org> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| kernel/futex.c | 12 ++++++++++-- |
| 1 file changed, 10 insertions(+), 2 deletions(-) |
| |
| diff --git a/kernel/futex.c b/kernel/futex.c |
| index c3ea6f2a6997..053d7be08be5 100644 |
| --- a/kernel/futex.c |
| +++ b/kernel/futex.c |
| @@ -1467,8 +1467,16 @@ static int futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr) |
| int oldval, ret; |
| |
| if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) { |
| - if (oparg < 0 || oparg > 31) |
| - return -EINVAL; |
| + if (oparg < 0 || oparg > 31) { |
| + char comm[sizeof(current->comm)]; |
| + /* |
| + * kill this print and return -EINVAL when userspace |
| + * is sane again |
| + */ |
| + pr_info_ratelimited("futex_wake_op: %s tries to shift op by %d; fix this program\n", |
| + get_task_comm(comm, current), oparg); |
| + oparg &= 31; |
| + } |
| oparg = 1 << oparg; |
| } |
| |
| -- |
| 2.17.1 |
| |