[RFC][PATCHSET] futex uaccess cleanups

[RFC][PATCHSET] futex uaccess cleanups

[Al Viro]

        As it is, arch_futex_atomic_op_inuser() has access_ok() done by
the (only) caller.  It would be better off closer to the actual user
memory access, as it is already done for futex_atomic_cmpxchg_inatomic().
And just as for futex_atomic_cmpxchg_inatomic() we can take
pagefault_{disable,enable}() into the caller.  Doing that brings
access_ok() and whatever an architecture needs to do to enable
the actual userland memory access into the same level; e.g. for x86
we can immediately convert them to user_access_begin/user_access_end
pair.
        Also in this series: removal of user_atomic_cmpxchg_inatomic().
The only remaining user had been futex_atomic_cmpxchg_inatomic() and
it doesn't require that kind of polymorphism.  It used to have callers
in MPX (and had been introduced for the sake of those), but MPX is
gone now and nobody else has ever made use of that primitive.

	Please, review.  This stuff lives in vfs.git#next.uaccess-3,
individual patches in followups.  The branch is based at #next.uaccess-2,
diffstat is
 arch/alpha/include/asm/futex.h      |  5 +-
 arch/arc/include/asm/futex.h        |  5 +-
 arch/arm/include/asm/futex.h        |  5 +-
 arch/arm64/include/asm/futex.h      |  5 +-
 arch/hexagon/include/asm/futex.h    |  5 +-
 arch/ia64/include/asm/futex.h       |  5 +-
 arch/microblaze/include/asm/futex.h |  5 +-
 arch/mips/include/asm/futex.h       |  5 +-
 arch/nds32/include/asm/futex.h      |  6 +--
 arch/openrisc/include/asm/futex.h   |  5 +-
 arch/parisc/include/asm/futex.h     |  2 -
 arch/powerpc/include/asm/futex.h    |  5 +-
 arch/riscv/include/asm/futex.h      |  5 +-
 arch/s390/include/asm/futex.h       |  2 -
 arch/sh/include/asm/futex.h         |  4 --
 arch/sparc/include/asm/futex_64.h   |  4 --
 arch/x86/include/asm/futex.h        | 38 ++++++++++-----
 arch/x86/include/asm/uaccess.h      | 93 -------------------------------------
 arch/xtensa/include/asm/futex.h     |  5 +-
 include/asm-generic/futex.h         |  2 -
 kernel/futex.c                      |  5 +-
 tools/objtool/check.c               |  1 +
 22 files changed, 58 insertions(+), 159 deletions(-)
Shortlog:
	futex: arch_futex_atomic_op_inuser() calling conventions change
	sh: no need of access_ok() in arch_futex_atomic_op_inuser()
	[parisc, s390, sparc64] no need for access_ok() in futex handling
	objtool: whitelist __sanitizer_cov_trace_switch()
	x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()
	generic arch_futex_atomic_op_inuser() doesn't need access_ok()
	x86: get rid of user_atomic_cmpxchg_inatomic()

[RFC][PATCH 1/7] futex: arch_futex_atomic_op_inuser() calling conventions change

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Move access_ok() in and pagefault_enable()/pagefault_disable() out.
Mechanical conversion only - some instances don't really need
a separate access_ok() at all (e.g. the ones only using
get_user()/put_user(), or architectures where access_ok()
is always true); we'll deal with that in followups.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/alpha/include/asm/futex.h      | 5 ++---
 arch/arc/include/asm/futex.h        | 5 +++--
 arch/arm/include/asm/futex.h        | 5 +++--
 arch/arm64/include/asm/futex.h      | 5 ++---
 arch/hexagon/include/asm/futex.h    | 5 ++---
 arch/ia64/include/asm/futex.h       | 5 ++---
 arch/microblaze/include/asm/futex.h | 5 ++---
 arch/mips/include/asm/futex.h       | 5 ++---
 arch/nds32/include/asm/futex.h      | 6 ++----
 arch/openrisc/include/asm/futex.h   | 5 ++---
 arch/parisc/include/asm/futex.h     | 5 +++--
 arch/powerpc/include/asm/futex.h    | 5 ++---
 arch/riscv/include/asm/futex.h      | 5 ++---
 arch/s390/include/asm/futex.h       | 4 ++--
 arch/sh/include/asm/futex.h         | 5 ++---
 arch/sparc/include/asm/futex_64.h   | 6 ++----
 arch/x86/include/asm/futex.h        | 5 ++---
 arch/xtensa/include/asm/futex.h     | 5 ++---
 include/asm-generic/futex.h         | 4 ++--
 kernel/futex.c                      | 5 ++---
 20 files changed, 43 insertions(+), 57 deletions(-)

diff --git a/arch/alpha/include/asm/futex.h b/arch/alpha/include/asm/futex.h
index bfd3c01038f8..da67afd578fd 100644
--- a/arch/alpha/include/asm/futex.h
+++ b/arch/alpha/include/asm/futex.h
@@ -31,7 +31,8 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 {
 	int oldval = 0, ret;
 
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -53,8 +54,6 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/arc/include/asm/futex.h b/arch/arc/include/asm/futex.h
index 9d0d070e6c22..607d1c16d4dd 100644
--- a/arch/arc/include/asm/futex.h
+++ b/arch/arc/include/asm/futex.h
@@ -75,10 +75,12 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 {
 	int oldval = 0, ret;
 
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
+
 #ifndef CONFIG_ARC_HAS_LLSC
 	preempt_disable();	/* to guarantee atomic r-m-w of futex op */
 #endif
-	pagefault_disable();
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -101,7 +103,6 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
 #ifndef CONFIG_ARC_HAS_LLSC
 	preempt_enable();
 #endif
diff --git a/arch/arm/include/asm/futex.h b/arch/arm/include/asm/futex.h
index 83c391b597d4..e133da303a98 100644
--- a/arch/arm/include/asm/futex.h
+++ b/arch/arm/include/asm/futex.h
@@ -134,10 +134,12 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 {
 	int oldval = 0, ret, tmp;
 
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
+
 #ifndef CONFIG_SMP
 	preempt_disable();
 #endif
-	pagefault_disable();
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -159,7 +161,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
 #ifndef CONFIG_SMP
 	preempt_enable();
 #endif
diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index 6cc26a127819..97f6a63810ec 100644
--- a/arch/arm64/include/asm/futex.h
+++ b/arch/arm64/include/asm/futex.h
@@ -48,7 +48,8 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *_uaddr)
 	int oldval = 0, ret, tmp;
 	u32 __user *uaddr = __uaccess_mask_ptr(_uaddr);
 
-	pagefault_disable();
+	if (!access_ok(_uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -75,8 +76,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *_uaddr)
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/hexagon/include/asm/futex.h b/arch/hexagon/include/asm/futex.h
index 0191f7c7193e..6b9c554aee78 100644
--- a/arch/hexagon/include/asm/futex.h
+++ b/arch/hexagon/include/asm/futex.h
@@ -36,7 +36,8 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 {
 	int oldval = 0, ret;
 
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -62,8 +63,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/ia64/include/asm/futex.h b/arch/ia64/include/asm/futex.h
index 2e106d462196..1db26b432d8c 100644
--- a/arch/ia64/include/asm/futex.h
+++ b/arch/ia64/include/asm/futex.h
@@ -50,7 +50,8 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 {
 	int oldval = 0, ret;
 
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -74,8 +75,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/microblaze/include/asm/futex.h b/arch/microblaze/include/asm/futex.h
index 8c90357e5983..86131ed84c9a 100644
--- a/arch/microblaze/include/asm/futex.h
+++ b/arch/microblaze/include/asm/futex.h
@@ -34,7 +34,8 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 {
 	int oldval = 0, ret;
 
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -56,8 +57,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/mips/include/asm/futex.h b/arch/mips/include/asm/futex.h
index 110220705e97..2bf8f6014579 100644
--- a/arch/mips/include/asm/futex.h
+++ b/arch/mips/include/asm/futex.h
@@ -89,7 +89,8 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 {
 	int oldval = 0, ret;
 
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -116,8 +117,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/nds32/include/asm/futex.h b/arch/nds32/include/asm/futex.h
index 5213c65c2e0b..4223f473bd36 100644
--- a/arch/nds32/include/asm/futex.h
+++ b/arch/nds32/include/asm/futex.h
@@ -66,8 +66,8 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 {
 	int oldval = 0, ret;
 
-
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 	switch (op) {
 	case FUTEX_OP_SET:
 		__futex_atomic_op("move	%0, %3", ret, oldval, tmp, uaddr,
@@ -93,8 +93,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/openrisc/include/asm/futex.h b/arch/openrisc/include/asm/futex.h
index fe894e6331ae..865e9cd0d97b 100644
--- a/arch/openrisc/include/asm/futex.h
+++ b/arch/openrisc/include/asm/futex.h
@@ -35,7 +35,8 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 {
 	int oldval = 0, ret;
 
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -57,8 +58,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/parisc/include/asm/futex.h b/arch/parisc/include/asm/futex.h
index d2c3e4106851..c10cc9010cc1 100644
--- a/arch/parisc/include/asm/futex.h
+++ b/arch/parisc/include/asm/futex.h
@@ -39,8 +39,10 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 	int oldval, ret;
 	u32 tmp;
 
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
+
 	_futex_spin_lock_irqsave(uaddr, &flags);
-	pagefault_disable();
 
 	ret = -EFAULT;
 	if (unlikely(get_user(oldval, uaddr) != 0))
@@ -73,7 +75,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 		ret = -EFAULT;
 
 out_pagefault_enable:
-	pagefault_enable();
 	_futex_spin_unlock_irqrestore(uaddr, &flags);
 
 	if (!ret)
diff --git a/arch/powerpc/include/asm/futex.h b/arch/powerpc/include/asm/futex.h
index bc7d9d06a6d9..f187bb5e524e 100644
--- a/arch/powerpc/include/asm/futex.h
+++ b/arch/powerpc/include/asm/futex.h
@@ -35,8 +35,9 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 {
 	int oldval = 0, ret;
 
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 	allow_read_write_user(uaddr, uaddr, sizeof(*uaddr));
-	pagefault_disable();
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -58,8 +59,6 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	*oval = oldval;
 
 	prevent_read_write_user(uaddr, uaddr, sizeof(*uaddr));
diff --git a/arch/riscv/include/asm/futex.h b/arch/riscv/include/asm/futex.h
index fdfaf7f3df7c..1b00badb9f87 100644
--- a/arch/riscv/include/asm/futex.h
+++ b/arch/riscv/include/asm/futex.h
@@ -46,7 +46,8 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 {
 	int oldval = 0, ret = 0;
 
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -73,8 +74,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/s390/include/asm/futex.h b/arch/s390/include/asm/futex.h
index 5e97a4353147..ed965c3ecd5b 100644
--- a/arch/s390/include/asm/futex.h
+++ b/arch/s390/include/asm/futex.h
@@ -28,8 +28,9 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 	int oldval = 0, newval, ret;
 	mm_segment_t old_fs;
 
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 	old_fs = enable_sacf_uaccess();
-	pagefault_disable();
 	switch (op) {
 	case FUTEX_OP_SET:
 		__futex_atomic_op("lr %2,%5\n",
@@ -54,7 +55,6 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 	default:
 		ret = -ENOSYS;
 	}
-	pagefault_enable();
 	disable_sacf_uaccess(old_fs);
 
 	if (!ret)
diff --git a/arch/sh/include/asm/futex.h b/arch/sh/include/asm/futex.h
index 3190ec89df81..324fa680b13d 100644
--- a/arch/sh/include/asm/futex.h
+++ b/arch/sh/include/asm/futex.h
@@ -34,7 +34,8 @@ static inline int arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval,
 	u32 oldval, newval, prev;
 	int ret;
 
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	do {
 		ret = get_user(oldval, uaddr);
@@ -67,8 +68,6 @@ static inline int arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval,
 		ret = futex_atomic_cmpxchg_inatomic(&prev, uaddr, oldval, newval);
 	} while (!ret && prev != oldval);
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/sparc/include/asm/futex_64.h b/arch/sparc/include/asm/futex_64.h
index 0865ce77ec00..84fffaaf59d3 100644
--- a/arch/sparc/include/asm/futex_64.h
+++ b/arch/sparc/include/asm/futex_64.h
@@ -35,11 +35,11 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 {
 	int oldval = 0, ret, tem;
 
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 	if (unlikely((((unsigned long) uaddr) & 0x3UL)))
 		return -EINVAL;
 
-	pagefault_disable();
-
 	switch (op) {
 	case FUTEX_OP_SET:
 		__futex_cas_op("mov\t%4, %1", ret, oldval, uaddr, oparg);
@@ -60,8 +60,6 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/x86/include/asm/futex.h b/arch/x86/include/asm/futex.h
index 13c83fe97988..6bcd1c1486d9 100644
--- a/arch/x86/include/asm/futex.h
+++ b/arch/x86/include/asm/futex.h
@@ -47,7 +47,8 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 {
 	int oldval = 0, ret, tem;
 
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -70,8 +71,6 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/arch/xtensa/include/asm/futex.h b/arch/xtensa/include/asm/futex.h
index 964611083224..a1a27b2ea460 100644
--- a/arch/xtensa/include/asm/futex.h
+++ b/arch/xtensa/include/asm/futex.h
@@ -72,7 +72,8 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 #if XCHAL_HAVE_S32C1I || XCHAL_HAVE_EXCLUSIVE
 	int oldval = 0, ret;
 
-	pagefault_disable();
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 
 	switch (op) {
 	case FUTEX_OP_SET:
@@ -99,8 +100,6 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 		ret = -ENOSYS;
 	}
 
-	pagefault_enable();
-
 	if (!ret)
 		*oval = oldval;
 
diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
index 02970b11f71f..3eab7ba912fc 100644
--- a/include/asm-generic/futex.h
+++ b/include/asm-generic/futex.h
@@ -33,8 +33,9 @@ arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
 	int oldval, ret;
 	u32 tmp;
 
+	if (!access_ok(uaddr, sizeof(u32)))
+		return -EFAULT;
 	preempt_disable();
-	pagefault_disable();
 
 	ret = -EFAULT;
 	if (unlikely(get_user(oldval, uaddr) != 0))
@@ -67,7 +68,6 @@ arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
 		ret = -EFAULT;
 
 out_pagefault_enable:
-	pagefault_enable();
 	preempt_enable();
 
 	if (ret == 0)
diff --git a/kernel/futex.c b/kernel/futex.c
index 0cf84c8664f2..7fdd2c949487 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1723,10 +1723,9 @@ static int futex_atomic_op_inuser(unsigned int encoded_op, u32 __user *uaddr)
 		oparg = 1 << oparg;
 	}
 
-	if (!access_ok(uaddr, sizeof(u32)))
-		return -EFAULT;
-
+	pagefault_disable();
 	ret = arch_futex_atomic_op_inuser(op, oparg, &oldval, uaddr);
+	pagefault_enable();
 	if (ret)
 		return ret;
 
-- 
2.11.0

[RFC][PATCH 2/7] sh: no need of access_ok() in arch_futex_atomic_op_inuser()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

everything it uses is doing access_ok() already

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/sh/include/asm/futex.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/sh/include/asm/futex.h b/arch/sh/include/asm/futex.h
index 324fa680b13d..b39cda09fb95 100644
--- a/arch/sh/include/asm/futex.h
+++ b/arch/sh/include/asm/futex.h
@@ -34,9 +34,6 @@ static inline int arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval,
 	u32 oldval, newval, prev;
 	int ret;
 
-	if (!access_ok(uaddr, sizeof(u32)))
-		return -EFAULT;
-
 	do {
 		ret = get_user(oldval, uaddr);
 
-- 
2.11.0

[RFC][PATCH 3/7] [parisc, s390, sparc64] no need for access_ok() in futex handling

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

access_ok() is always true on those

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/parisc/include/asm/futex.h   | 3 ---
 arch/s390/include/asm/futex.h     | 2 --
 arch/sparc/include/asm/futex_64.h | 2 --
 3 files changed, 7 deletions(-)

diff --git a/arch/parisc/include/asm/futex.h b/arch/parisc/include/asm/futex.h
index c10cc9010cc1..c459f656c8c3 100644
--- a/arch/parisc/include/asm/futex.h
+++ b/arch/parisc/include/asm/futex.h
@@ -39,9 +39,6 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
 	int oldval, ret;
 	u32 tmp;
 
-	if (!access_ok(uaddr, sizeof(u32)))
-		return -EFAULT;
-
 	_futex_spin_lock_irqsave(uaddr, &flags);
 
 	ret = -EFAULT;
diff --git a/arch/s390/include/asm/futex.h b/arch/s390/include/asm/futex.h
index ed965c3ecd5b..26f9144562c9 100644
--- a/arch/s390/include/asm/futex.h
+++ b/arch/s390/include/asm/futex.h
@@ -28,8 +28,6 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 	int oldval = 0, newval, ret;
 	mm_segment_t old_fs;
 
-	if (!access_ok(uaddr, sizeof(u32)))
-		return -EFAULT;
 	old_fs = enable_sacf_uaccess();
 	switch (op) {
 	case FUTEX_OP_SET:
diff --git a/arch/sparc/include/asm/futex_64.h b/arch/sparc/include/asm/futex_64.h
index 84fffaaf59d3..72de967318d7 100644
--- a/arch/sparc/include/asm/futex_64.h
+++ b/arch/sparc/include/asm/futex_64.h
@@ -35,8 +35,6 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 {
 	int oldval = 0, ret, tem;
 
-	if (!access_ok(uaddr, sizeof(u32)))
-		return -EFAULT;
 	if (unlikely((((unsigned long) uaddr) & 0x3UL)))
 		return -EINVAL;
 
-- 
2.11.0

[RFC][PATCH 4/7] objtool: whitelist __sanitizer_cov_trace_switch()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

it's not really different from e.g. __sanitizer_cov_trace_cmp4();
as it is, the switches that generate an array of labels get
rejected by objtool, while slightly different set of cases
that gets compiled into a series of comparisons is accepted.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 tools/objtool/check.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4768d91c6d68..3667c5d7453a 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -478,6 +478,7 @@ static const char *uaccess_safe_builtin[] = {
 	"__sanitizer_cov_trace_cmp2",
 	"__sanitizer_cov_trace_cmp4",
 	"__sanitizer_cov_trace_cmp8",
+	"__sanitizer_cov_trace_switch",
 	/* UBSAN */
 	"ubsan_type_mismatch_common",
 	"__ubsan_handle_type_mismatch",
-- 
2.11.0

[RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Lift stac/clac pairs from __futex_atomic_op{1,2} into arch_futex_atomic_op_inuser(),
fold them with access_ok() in there.  The switch in arch_futex_atomic_op_inuser()
is what has required the previous (objtool) commit...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/x86/include/asm/futex.h | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/futex.h b/arch/x86/include/asm/futex.h
index 6bcd1c1486d9..cf6e57fb5b22 100644
--- a/arch/x86/include/asm/futex.h
+++ b/arch/x86/include/asm/futex.h
@@ -13,9 +13,8 @@
 #include <asm/smap.h>
 
 #define __futex_atomic_op1(insn, ret, oldval, uaddr, oparg)	\
-	asm volatile("\t" ASM_STAC "\n"				\
-		     "1:\t" insn "\n"				\
-		     "2:\t" ASM_CLAC "\n"			\
+	asm volatile("1:\t" insn "\n"				\
+		     "2:\n"					\
 		     "\t.section .fixup,\"ax\"\n"		\
 		     "3:\tmov\t%3, %1\n"			\
 		     "\tjmp\t2b\n"				\
@@ -25,13 +24,12 @@
 		     : "i" (-EFAULT), "0" (oparg), "1" (0))
 
 #define __futex_atomic_op2(insn, ret, oldval, uaddr, oparg)	\
-	asm volatile("\t" ASM_STAC "\n"				\
-		     "1:\tmovl	%2, %0\n"			\
+	asm volatile("1:\tmovl	%2, %0\n"			\
 		     "\tmovl\t%0, %3\n"				\
 		     "\t" insn "\n"				\
 		     "2:\t" LOCK_PREFIX "cmpxchgl %3, %2\n"	\
 		     "\tjnz\t1b\n"				\
-		     "3:\t" ASM_CLAC "\n"			\
+		     "3:\n"					\
 		     "\t.section .fixup,\"ax\"\n"		\
 		     "4:\tmov\t%5, %1\n"			\
 		     "\tjmp\t3b\n"				\
@@ -42,12 +40,12 @@
 		       "+m" (*uaddr), "=&r" (tem)		\
 		     : "r" (oparg), "i" (-EFAULT), "1" (0))
 
-static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
+static __always_inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 		u32 __user *uaddr)
 {
 	int oldval = 0, ret, tem;
 
-	if (!access_ok(uaddr, sizeof(u32)))
+	if (!user_access_begin(uaddr, sizeof(u32)))
 		return -EFAULT;
 
 	switch (op) {
@@ -70,6 +68,7 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 	default:
 		ret = -ENOSYS;
 	}
+	user_access_end();
 
 	if (!ret)
 		*oval = oldval;
-- 
2.11.0

[RFC][PATCH 6/7] generic arch_futex_atomic_op_inuser() doesn't need access_ok()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

uses get_user() and put_user() for memory accesses

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/asm-generic/futex.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/asm-generic/futex.h b/include/asm-generic/futex.h
index 3eab7ba912fc..f4c3470480c7 100644
--- a/include/asm-generic/futex.h
+++ b/include/asm-generic/futex.h
@@ -33,8 +33,6 @@ arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
 	int oldval, ret;
 	u32 tmp;
 
-	if (!access_ok(uaddr, sizeof(u32)))
-		return -EFAULT;
 	preempt_disable();
 
 	ret = -EFAULT;
-- 
2.11.0

[RFC][PATCH 7/7] x86: get rid of user_atomic_cmpxchg_inatomic()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Only one user left; the thing had been made polymorphic back in 2013
for the sake of MPX.  No point keeping it now that MPX is gone.
Convert futex_atomic_cmpxchg_inatomic() to user_access_{begin,end}()
while we are at it.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/x86/include/asm/futex.h   | 20 ++++++++-
 arch/x86/include/asm/uaccess.h | 93 ------------------------------------------
 2 files changed, 19 insertions(+), 94 deletions(-)

diff --git a/arch/x86/include/asm/futex.h b/arch/x86/include/asm/futex.h
index cf6e57fb5b22..109cb758a36e 100644
--- a/arch/x86/include/asm/futex.h
+++ b/arch/x86/include/asm/futex.h
@@ -79,7 +79,25 @@ static __always_inline int arch_futex_atomic_op_inuser(int op, int oparg, int *o
 static inline int futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
 						u32 oldval, u32 newval)
 {
-	return user_atomic_cmpxchg_inatomic(uval, uaddr, oldval, newval);
+	int ret = 0;
+
+	if (!user_access_begin(uaddr, sizeof(u32)))
+		return -EFAULT;
+	asm volatile("\n"
+		"1:\t" LOCK_PREFIX "cmpxchgl %4, %2\n"
+		"2:\n"
+		"\t.section .fixup, \"ax\"\n"
+		"3:\tmov     %3, %0\n"
+		"\tjmp     2b\n"
+		"\t.previous\n"
+		_ASM_EXTABLE_UA(1b, 3b)
+		: "+r" (ret), "=a" (oldval), "+m" (*uaddr)
+		: "i" (-EFAULT), "r" (newval), "1" (oldval)
+		: "memory"
+	);
+	user_access_end();
+	*uval = oldval;
+	return ret;
 }
 
 #endif
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index d11662f753d2..c8247a84244b 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -453,99 +453,6 @@ extern __must_check long strnlen_user(const char __user *str, long n);
 unsigned long __must_check clear_user(void __user *mem, unsigned long len);
 unsigned long __must_check __clear_user(void __user *mem, unsigned long len);
 
-extern void __cmpxchg_wrong_size(void)
-	__compiletime_error("Bad argument size for cmpxchg");
-
-#define __user_atomic_cmpxchg_inatomic(uval, ptr, old, new, size)	\
-({									\
-	int __ret = 0;							\
-	__typeof__(*(ptr)) __old = (old);				\
-	__typeof__(*(ptr)) __new = (new);				\
-	__uaccess_begin_nospec();					\
-	switch (size) {							\
-	case 1:								\
-	{								\
-		asm volatile("\n"					\
-			"1:\t" LOCK_PREFIX "cmpxchgb %4, %2\n"		\
-			"2:\n"						\
-			"\t.section .fixup, \"ax\"\n"			\
-			"3:\tmov     %3, %0\n"				\
-			"\tjmp     2b\n"				\
-			"\t.previous\n"					\
-			_ASM_EXTABLE_UA(1b, 3b)				\
-			: "+r" (__ret), "=a" (__old), "+m" (*(ptr))	\
-			: "i" (-EFAULT), "q" (__new), "1" (__old)	\
-			: "memory"					\
-		);							\
-		break;							\
-	}								\
-	case 2:								\
-	{								\
-		asm volatile("\n"					\
-			"1:\t" LOCK_PREFIX "cmpxchgw %4, %2\n"		\
-			"2:\n"						\
-			"\t.section .fixup, \"ax\"\n"			\
-			"3:\tmov     %3, %0\n"				\
-			"\tjmp     2b\n"				\
-			"\t.previous\n"					\
-			_ASM_EXTABLE_UA(1b, 3b)				\
-			: "+r" (__ret), "=a" (__old), "+m" (*(ptr))	\
-			: "i" (-EFAULT), "r" (__new), "1" (__old)	\
-			: "memory"					\
-		);							\
-		break;							\
-	}								\
-	case 4:								\
-	{								\
-		asm volatile("\n"					\
-			"1:\t" LOCK_PREFIX "cmpxchgl %4, %2\n"		\
-			"2:\n"						\
-			"\t.section .fixup, \"ax\"\n"			\
-			"3:\tmov     %3, %0\n"				\
-			"\tjmp     2b\n"				\
-			"\t.previous\n"					\
-			_ASM_EXTABLE_UA(1b, 3b)				\
-			: "+r" (__ret), "=a" (__old), "+m" (*(ptr))	\
-			: "i" (-EFAULT), "r" (__new), "1" (__old)	\
-			: "memory"					\
-		);							\
-		break;							\
-	}								\
-	case 8:								\
-	{								\
-		if (!IS_ENABLED(CONFIG_X86_64))				\
-			__cmpxchg_wrong_size();				\
-									\
-		asm volatile("\n"					\
-			"1:\t" LOCK_PREFIX "cmpxchgq %4, %2\n"		\
-			"2:\n"						\
-			"\t.section .fixup, \"ax\"\n"			\
-			"3:\tmov     %3, %0\n"				\
-			"\tjmp     2b\n"				\
-			"\t.previous\n"					\
-			_ASM_EXTABLE_UA(1b, 3b)				\
-			: "+r" (__ret), "=a" (__old), "+m" (*(ptr))	\
-			: "i" (-EFAULT), "r" (__new), "1" (__old)	\
-			: "memory"					\
-		);							\
-		break;							\
-	}								\
-	default:							\
-		__cmpxchg_wrong_size();					\
-	}								\
-	__uaccess_end();						\
-	*(uval) = __old;						\
-	__ret;								\
-})
-
-#define user_atomic_cmpxchg_inatomic(uval, ptr, old, new)		\
-({									\
-	access_ok((ptr), sizeof(*(ptr))) ?		\
-		__user_atomic_cmpxchg_inatomic((uval), (ptr),		\
-				(old), (new), sizeof(*(ptr))) :		\
-		-EFAULT;						\
-})
-
 /*
  * movsl can be slow when source and dest are not both 8-byte aligned
  */
-- 
2.11.0

Re: [RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Linus Torvalds]

On Mon, Mar 23, 2020 at 11:53 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> From: Al Viro <viro@zeniv.linux.org.uk>
>
> Lift stac/clac pairs from __futex_atomic_op{1,2} into arch_futex_atomic_op_inuser(),
> fold them with access_ok() in there.

So this is a deep internal macro and already has the double
underscore, but I'm inclined to say "add the unsafe here too" for
those __futex_atomic_opX() macros that are now called inside that
user_access_begin/end region.

And wouldn't it be lovely to get rid of the error return thing, and
pass in a label instead, the way "usafe_get/put_user()" works too?
That might be a separate patch from the "reorg" thing, though.

             Linus

Re: [RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Al Viro]

On Mon, Mar 23, 2020 at 12:06:39PM -0700, Linus Torvalds wrote:
> On Mon, Mar 23, 2020 at 11:53 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > From: Al Viro <viro@zeniv.linux.org.uk>
> >
> > Lift stac/clac pairs from __futex_atomic_op{1,2} into arch_futex_atomic_op_inuser(),
> > fold them with access_ok() in there.
> 
> So this is a deep internal macro and already has the double
> underscore, but I'm inclined to say "add the unsafe here too" for
> those __futex_atomic_opX() macros that are now called inside that
> user_access_begin/end region.
> 
> And wouldn't it be lovely to get rid of the error return thing, and
> pass in a label instead, the way "usafe_get/put_user()" works too?
> That might be a separate patch from the "reorg" thing, though.

Umm...
#define __futex_atomic_op1(insn, ret, oldval, uaddr, oparg)     \
        asm volatile("1:\t" insn "\n"                           \
                     "2:\n"                                     \
                     "\t.section .fixup,\"ax\"\n"               \
                     "3:\tmov\t%3, %1\n"                        \
                     "\tjmp\t2b\n"                              \
                     "\t.previous\n"                            \
                     _ASM_EXTABLE_UA(1b, 3b)                    \
                     : "=r" (oldval), "=r" (ret), "+m" (*uaddr) \
                     : "i" (-EFAULT), "0" (oparg), "1" (0))

OK, ret wouldn't be in the list of outputs that way and
*uaddr could become an input (we only care about the address,
same as for put_user), but oldval is a genuine output -
insn is xchgl or lock xaddl, and we obviously care about the
old value fetched by it.  The same goes for
#define __futex_atomic_op2(insn, ret, oldval, uaddr, oparg)     \
        asm volatile("1:\tmovl  %2, %0\n"                       \
                     "\tmovl\t%0, %3\n"                         \
                     "\t" insn "\n"                             \
                     "2:\t" LOCK_PREFIX "cmpxchgl %3, %2\n"     \
                     "\tjnz\t1b\n"                              \
                     "3:\n"                                     \
                     "\t.section .fixup,\"ax\"\n"               \
                     "4:\tmov\t%5, %1\n"                        \
                     "\tjmp\t3b\n"                              \
                     "\t.previous\n"                            \
                     _ASM_EXTABLE_UA(1b, 4b)                    \
                     _ASM_EXTABLE_UA(2b, 4b)                    \
                     : "=&a" (oldval), "=&r" (ret),             \
                       "+m" (*uaddr), "=&r" (tem)               \
                     : "r" (oparg), "i" (-EFAULT), "1" (0))
- oldval is calculated by that thing and arch_futex_atomic_op_inuser()
ends up storing it in *oval.  And moving that assignment into
the inline asm will simply put *oval into the output list,
won't it?

How would you work around that?

Re: [RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Linus Torvalds]

On Mon, Mar 23, 2020 at 7:08 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> > And wouldn't it be lovely to get rid of the error return thing, and
> > pass in a label instead, the way "usafe_get/put_user()" works too?
> > That might be a separate patch from the "reorg" thing, though.
>
> OK, ret wouldn't be in the list of outputs that way and
> *uaddr could become an input (we only care about the address,
> same as for put_user), but oldval is a genuine output -

Yes, initially we'd have to do the "jump to label" inside the macro,
because gcc doesn't support asm goto with outputs.

But that's no different from "unsafe_get_user()". We still pass it a
label, even though we can't use it in the inline asm.

Yet.

I have patches to make it work with newer versions of clang, and I
hope that gcc will eventually also accept the semantics of "asm goto
with outputs only has the output on the fallthrough".

So _currently_ it would be only syntactic sugar: moving the error
handling inside the macro, and making it syntactically match
unsafe_get_user().

But long term is would allow us to generate better code too.

                Linus

Re: [RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Al Viro]

On Tue, Mar 24, 2020 at 09:19:15AM -0700, Linus Torvalds wrote:
> On Mon, Mar 23, 2020 at 7:08 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > > And wouldn't it be lovely to get rid of the error return thing, and
> > > pass in a label instead, the way "usafe_get/put_user()" works too?
> > > That might be a separate patch from the "reorg" thing, though.
> >
> > OK, ret wouldn't be in the list of outputs that way and
> > *uaddr could become an input (we only care about the address,
> > same as for put_user), but oldval is a genuine output -
> 
> Yes, initially we'd have to do the "jump to label" inside the macro,
> because gcc doesn't support asm goto with outputs.
> 
> But that's no different from "unsafe_get_user()". We still pass it a
> label, even though we can't use it in the inline asm.
> 
> Yet.
> 
> I have patches to make it work with newer versions of clang, and I
> hope that gcc will eventually also accept the semantics of "asm goto
> with outputs only has the output on the fallthrough".
> 
> So _currently_ it would be only syntactic sugar: moving the error
> handling inside the macro, and making it syntactically match
> unsafe_get_user().
> 
> But long term is would allow us to generate better code too.

OK...  BTW, I'd been trying to recall the reasons for the way
__futex_atomic_op2() loop is done; ISTR some discussion along
the lines of cacheline ping-pong prevention, but I'd been unable
to reconstruct enough details to find it and I'm not sure it
hadn't been about some other code ;-/

What we have there (fault handling aside) is
loop:	eax = *uaddr;
	v = eax | oparg;
	lock cmpxchg v, *uaddr
	if (!zf)
		goto loop;
	oldval = eax;
Why do we bother with reload?  That cmpxchg is, after all,
	t = *uaddr;
	zf = (t == eax);
	*uaddr = zf ? v : t;
	eax = t;
so what would be wrong with doing
	eax = *uaddr;
loop:	v = eax | oparg;
	lock cmpxchg v, *uaddr
	if (!zf)
		goto loop;
	oldval = eax;
instead?

Re: [RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Linus Torvalds]

On Tue, Mar 24, 2020 at 1:45 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> OK...  BTW, I'd been trying to recall the reasons for the way
> __futex_atomic_op2() loop is done; ISTR some discussion along
> the lines of cacheline ping-pong prevention, but I'd been unable
> to reconstruct enough details to find it and I'm not sure it
> hadn't been about some other code ;-/

No, that doesn't look like any cacheline advantage I can think of -
quite the reverse.

I suspect it's just lazy code, with the reload being unnecessary. Or
it might be written that way because you avoid an extra variable.

In fact, from a cacheline optimization standpoint, there are
advantages to not doing the load even on the initial run: if you know
a certain value is particularly likely, there are advantages to just
_assuming_ that value, rather than loading it. So no initial load at
all, and just initialize the first value to the likely case.

That can avoid an unnecessary "load for shared ownership" cacheline
state transition (since the cmpxchg will want to turn it into an
exclusive modified cacheline anyway).

But I don't think that optimization is likely the case here, and
you're right, the loop would be better written with the initial load
outside the loop.

           Linus

Re: [RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Al Viro]

On Tue, Mar 24, 2020 at 01:57:19PM -0700, Linus Torvalds wrote:
> On Tue, Mar 24, 2020 at 1:45 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > OK...  BTW, I'd been trying to recall the reasons for the way
> > __futex_atomic_op2() loop is done; ISTR some discussion along
> > the lines of cacheline ping-pong prevention, but I'd been unable
> > to reconstruct enough details to find it and I'm not sure it
> > hadn't been about some other code ;-/
> 
> No, that doesn't look like any cacheline advantage I can think of -
> quite the reverse.
> 
> I suspect it's just lazy code, with the reload being unnecessary. Or
> it might be written that way because you avoid an extra variable.
> 
> In fact, from a cacheline optimization standpoint, there are
> advantages to not doing the load even on the initial run: if you know
> a certain value is particularly likely, there are advantages to just
> _assuming_ that value, rather than loading it. So no initial load at
> all, and just initialize the first value to the likely case.
> 
> That can avoid an unnecessary "load for shared ownership" cacheline
> state transition (since the cmpxchg will want to turn it into an
> exclusive modified cacheline anyway).
> 
> But I don't think that optimization is likely the case here, and
> you're right, the loop would be better written with the initial load
> outside the loop.

OK, updated branch is in the same place; changes: __futex_atomic_op{1,2}
turned into unsafe_atomic_op{1,2}, with "goto on error" folded into those.
And pointless reload removed from cmpxchg loop in unsafe_atomic_op2().
Diffstat:
 arch/alpha/include/asm/futex.h      |  5 +-
 arch/arc/include/asm/futex.h        |  5 +-
 arch/arm/include/asm/futex.h        |  5 +-
 arch/arm64/include/asm/futex.h      |  5 +-
 arch/hexagon/include/asm/futex.h    |  5 +-
 arch/ia64/include/asm/futex.h       |  5 +-
 arch/microblaze/include/asm/futex.h |  5 +-
 arch/mips/include/asm/futex.h       |  5 +-
 arch/nds32/include/asm/futex.h      |  6 +--
 arch/openrisc/include/asm/futex.h   |  5 +-
 arch/parisc/include/asm/futex.h     |  2 -
 arch/powerpc/include/asm/futex.h    |  5 +-
 arch/riscv/include/asm/futex.h      |  5 +-
 arch/s390/include/asm/futex.h       |  2 -
 arch/sh/include/asm/futex.h         |  4 --
 arch/sparc/include/asm/futex_64.h   |  4 --
 arch/x86/include/asm/futex.h        | 97 ++++++++++++++++++++++++-------------
 arch/x86/include/asm/uaccess.h      | 93 -----------------------------------
 arch/xtensa/include/asm/futex.h     |  5 +-
 include/asm-generic/futex.h         |  2 -
 kernel/futex.c                      |  5 +-
 tools/objtool/check.c               |  1 +
 22 files changed, 93 insertions(+), 183 deletions(-)

Sorry about the fuckup when sending that patchset ;-/  It ended up cc'd to
x86 list instead of the futex one; Message-Id of the beginning of the
thread is <20200327022836.881203-1-viro@ZenIV.linux.org.uk>.

Re: [RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Linus Torvalds]

Your threads are really hard to follow when you reply to an email in
the middle of the previous series and make that reply be the cover
letter for the next one.

Anyway, apart from the problem with the futex series that I replied
to, I don't see anything wrong.

I will not boot with both series applied in a test-tree.

Wish me luck.

           Linus

Re: [RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Linus Torvalds]

On Thu, Mar 26, 2020 at 8:42 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I will not boot with both series applied in a test-tree.
         ^^^ now
> Wish me luck.

Seems to work for me.

That's with the futex bug fixed. Not that it looks like it would have
mattered except for the (unlikely) exception case, so my testing is
meaningless.

But on the whole it seems to work for me, and I can't see anything
else wrong than that incorrect branch target in the futex rewrite.

            Linus

Re: [RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Linus Torvalds]

On Thu, Mar 26, 2020 at 8:49 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Seems to work for me.
>
> That's with the futex bug fixed. Not that it looks like it would have
> mattered except for the (unlikely) exception case, so my testing is
> meaningless.

Hmm. Doing a "perf" run, I only noticed after-the-fact that I got this:

  WARNING: stack recursion on stack type 4
  WARNING: can't dereference registers at 0000000079a3d9c5 for ip
swapgs_restore_regs_and_return_to_usermode+0x25/0x80

that may not be due to any of the uaccess or futex changes, though, it
smells like just bad luck.

Josh?

This may also be related to the fact that I've been building my
test-boot kernels with clang for the last couple of months,

That "swapgs_restore_regs_and_return_to_usermode+0x25" location is the

        pushq  0x28(%rdi)

instruction. That's this:

        movq    %rsp, %rdi
        movq    PER_CPU_VAR(cpu_tss_rw + TSS_sp0), %rsp

        /* Copy the IRET frame to the trampoline stack. */
        pushq   6*8(%rdi)       /* SS */
--->    pushq   5*8(%rdi)       /* RSP */
        pushq   4*8(%rdi)       /* EFLAGS */
        pushq   3*8(%rdi)       /* CS */
        pushq   2*8(%rdi)       /* RIP */

and yeah, at this point the stack is obviously a mess, so I'm not
surprised that it might cause confusion for unwinding..

              Linus

Re: [RFC][PATCH 5/7] x86: convert arch_futex_atomic_op_inuser() to user_access_begin/user_access_end()

[Josh Poimboeuf]

On Thu, Mar 26, 2020 at 09:03:41PM -0700, Linus Torvalds wrote:
> On Thu, Mar 26, 2020 at 8:49 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Seems to work for me.
> >
> > That's with the futex bug fixed. Not that it looks like it would have
> > mattered except for the (unlikely) exception case, so my testing is
> > meaningless.
> 
> Hmm. Doing a "perf" run, I only noticed after-the-fact that I got this:
> 
>   WARNING: stack recursion on stack type 4
>   WARNING: can't dereference registers at 0000000079a3d9c5 for ip
> swapgs_restore_regs_and_return_to_usermode+0x25/0x80
> 
> that may not be due to any of the uaccess or futex changes, though, it
> smells like just bad luck.
> 
> Josh?
> 
> This may also be related to the fact that I've been building my
> test-boot kernels with clang for the last couple of months,
> 
> That "swapgs_restore_regs_and_return_to_usermode+0x25" location is the
> 
>         pushq  0x28(%rdi)
> 
> instruction. That's this:
> 
>         movq    %rsp, %rdi
>         movq    PER_CPU_VAR(cpu_tss_rw + TSS_sp0), %rsp
> 
>         /* Copy the IRET frame to the trampoline stack. */
>         pushq   6*8(%rdi)       /* SS */
> --->    pushq   5*8(%rdi)       /* RSP */
>         pushq   4*8(%rdi)       /* EFLAGS */
>         pushq   3*8(%rdi)       /* CS */
>         pushq   2*8(%rdi)       /* RIP */
> 
> and yeah, at this point the stack is obviously a mess, so I'm not
> surprised that it might cause confusion for unwinding..

You did indeed get unlucky, and that's the correct diagnosis.  It's
pretty harmless.

  https://lkml.kernel.org/r/58c05bf0a9f06ac7f2ed6df5e369d3276ccec33c.1584033751.git.jpoimboe@redhat.com

Working on a v2, I'll add you to the already excessive Reported-by list
:-)

-- 
Josh