From 81da9f87ad9d5b77014f0d11946ffbc887bdec70 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Thu, 24 Aug 2017 09:31:05 +0200 Subject: futex: Remove duplicated code and fix undefined behaviour commit 30d6e0a4190d37740e9447e4e4815f06992dd8c3 upstream. There is code duplicated over all architecture's headers for futex_atomic_op_inuser. Namely op decoding, access_ok check for uaddr, and comparison of the result. Remove this duplication and leave up to the arches only the needed assembly which is now in arch_futex_atomic_op_inuser. This effectively distributes the Will Deacon's arm64 fix for undefined behaviour reported by UBSAN to all architectures. The fix was done in commit 5f16a046f8e1 (arm64: futex: Fix undefined behaviour with FUTEX_OP_OPARG_SHIFT usage). Look there for an example dump. And as suggested by Thomas, check for negative oparg too, because it was also reported to cause undefined behaviour report. Note that s390 removed access_ok check in d12a29703 ("s390/uaccess: remove pointless access_ok() checks") as access_ok there returns true. We introduce it back to the helper for the sake of simplicity (it gets optimized away anyway). Signed-off-by: Jiri Slaby Signed-off-by: Thomas Gleixner Acked-by: Russell King Acked-by: Michael Ellerman (powerpc) Acked-by: Heiko Carstens [s390] Acked-by: Chris Metcalf [for tile] Reviewed-by: Darren Hart (VMware) Reviewed-by: Will Deacon [core/arm64] Cc: linux-mips@linux-mips.org Cc: Rich Felker Cc: linux-ia64@vger.kernel.org Cc: linux-sh@vger.kernel.org Cc: peterz@infradead.org Cc: Benjamin Herrenschmidt Cc: Max Filippov Cc: Paul Mackerras Cc: sparclinux@vger.kernel.org Cc: Jonas Bonn Cc: linux-s390@vger.kernel.org Cc: linux-arch@vger.kernel.org Cc: Yoshinori Sato Cc: linux-hexagon@vger.kernel.org Cc: Helge Deller Cc: "James E.J. Bottomley" Cc: Catalin Marinas Cc: Matt Turner Cc: linux-snps-arc@lists.infradead.org Cc: Fenghua Yu Cc: Arnd Bergmann Cc: linux-xtensa@linux-xtensa.org Cc: Stefan Kristiansson Cc: openrisc@lists.librecores.org Cc: Ivan Kokshaysky Cc: Stafford Horne Cc: linux-arm-kernel@lists.infradead.org Cc: Richard Henderson Cc: Chris Zankel Cc: Michal Simek Cc: Tony Luck Cc: linux-parisc@vger.kernel.org Cc: Vineet Gupta Cc: Ralf Baechle Cc: Richard Kuo Cc: linux-alpha@vger.kernel.org Cc: Martin Schwidefsky Cc: linuxppc-dev@lists.ozlabs.org Cc: "David S. Miller" Link: http://lkml.kernel.org/r/20170824073105.3901-1-jslaby@suse.cz Cc: Ben Hutchings Signed-off-by: Greg Kroah-Hartman --- include/asm-generic/futex.h | 50 ++++++++------------------------------------- 1 file changed, 9 insertions(+), 41 deletions(-) (limited to 'include') diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h index bf2d34c9d804..f0d8b1c51343 100644 --- a/include/asm-generic/futex.h +++ b/include/asm-generic/futex.h @@ -13,7 +13,7 @@ */ /** - * futex_atomic_op_inuser() - Atomic arithmetic operation with constant + * arch_futex_atomic_op_inuser() - Atomic arithmetic operation with constant * argument and comparison of the previous * futex value with another constant. * @@ -25,18 +25,11 @@ * <0 - On error */ static inline int -futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr) +arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr) { - int op = (encoded_op >> 28) & 7; - int cmp = (encoded_op >> 24) & 15; - int oparg = (encoded_op << 8) >> 20; - int cmparg = (encoded_op << 20) >> 20; int oldval, ret; u32 tmp; - if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) - oparg = 1 << oparg; - preempt_disable(); pagefault_disable(); @@ -74,17 +67,9 @@ out_pagefault_enable: pagefault_enable(); preempt_enable(); - if (ret == 0) { - switch (cmp) { - case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break; - case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break; - case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break; - case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break; - case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break; - case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break; - default: ret = -ENOSYS; - } - } + if (ret == 0) + *oval = oldval; + return ret; } @@ -126,18 +111,9 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr, #else static inline int -futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr) +arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr) { - int op = (encoded_op >> 28) & 7; - int cmp = (encoded_op >> 24) & 15; - int oparg = (encoded_op << 8) >> 20; - int cmparg = (encoded_op << 20) >> 20; int oldval = 0, ret; - if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) - oparg = 1 << oparg; - - if (! access_ok (VERIFY_WRITE, uaddr, sizeof(u32))) - return -EFAULT; pagefault_disable(); @@ -153,17 +129,9 @@ futex_atomic_op_inuser (int encoded_op, u32 __user *uaddr) pagefault_enable(); - if (!ret) { - switch (cmp) { - case FUTEX_OP_CMP_EQ: ret = (oldval == cmparg); break; - case FUTEX_OP_CMP_NE: ret = (oldval != cmparg); break; - case FUTEX_OP_CMP_LT: ret = (oldval < cmparg); break; - case FUTEX_OP_CMP_GE: ret = (oldval >= cmparg); break; - case FUTEX_OP_CMP_LE: ret = (oldval <= cmparg); break; - case FUTEX_OP_CMP_GT: ret = (oldval > cmparg); break; - default: ret = -ENOSYS; - } - } + if (!ret) + *oval = oldval; + return ret; } -- cgit v1.2.3