[patch V2 0/6] uaccess: Provide and use scopes for user masked access

[patch V2 0/6] uaccess: Provide and use scopes for user masked access

[Thomas Gleixner]

This is a follow up on the initial V1 to make the masked user access more
accessible:

   https://lore.kernel.org/r/20250813150610.521355442@linutronix.de

After reading through the discussions in the V1 thread, I sat down
and thought about this some more.

My initial reason to tackle this was that the usage pattern is tedious:

	if (can_do_masked_user_access())
		from = masked_user_read_access_begin((from));
	else if (!user_read_access_begin(from, sizeof(*from)))
		return -EFAULT;
	unsafe_get_user(val, from, Efault);
	user_read_access_end();
	return 0;
Efault:
	user_read_access_end();
	return -EFAULT;

This obviously has some interesting ways to get it wrong and after a while
I came to the conclusion that this really begs for a scope based
implementation with automatic cleanup.

After quite some frustrating fights with macro limitations, I finally came
up with a scheme, which provides scoped guards for this.

This allows to implement the above as:

	scoped_masked_user_read_access(ptr, return -EFAULT,
		scoped_get_user(val, ptr); );
	return 0;

The scope hides the masked user magic and ensures that the proper
access_end() variant is invoked when leaving the scope.

It provides a scope local fault label ('scope_fault:'), which has to
be used by the user accesses within the scope. The label is placed
before the exit code ('return -EFAULT' in the above example)

The provided scoped_get/put_user() macros use 'scope_fault'
internally, i.e. they expand to

    unsafe_get/put_user(val, ptr, scope_fault)

Obvioulsly nothing prevents using unsafe_get/put_user() within the scope
and supplying a wrong label:

	scoped_masked_user_read_access(ptr, return -EFAULT,
		unsafe_get_user(val, ptr, fail); );
	return 0;
fail:
	return -EFAULT;

This bug is caught at least by clang, but GCC happily jumps outside the
cleanup scope.

Using a dedicated label is possible as long as it is within the scope:

	scoped_masked_user_read_access(ptr, return -EFAULT, {
		unsafe_get_user(*val, ptr, fail);
		return 0;
	fail:
		*val = 99;
	});
	return -EFAULT;

That example does not make a lot of sense, but at least it's correct :)

In that case the error code 'return -EFAULT' is only used when the
architecture does not support masked access and user_access_begin()
fails. That error exit code must obviously be run _before_ the cleanup
scope starts because user_access_begin() does not enable user access
on failure.

Unfortunately clang < version 17 has issues with scope local labels, which
means that ASM goto needs to be disabled for clang < 17 to make this
work. GCC seems to be doing fine (except for not detecting the above label
scope bug).

The user pointer 'ptr' is aliased with the eventually modified pointer
within the scope, which means that the following would work correctly:

	bool result = true;

	scoped_masked_user_read_access(ptr, result = false,
		scoped_get_user(val, ptr); );

        if (!result) {
	   	// ptr is unmodified even when masking modified it
		// within the scope, so do_magic() gets the original
		// value.
		do_magic(ptr);
	}

Not sure whether it matters. The aliasing is not really required for the
code to function and could be removed if there is a real argument against
it.

Looking at the compiler output for this scope magic.

bool set_usr_val(u32 val, u32 *ptr)
{
	scoped_masked_user_read_access(ptr, return false,
		scoped_get_user(val, ptr); );
	return true;
}

On x86 with masked access and ASM goto supported clang-19 compiles
it to:

0000000000000b60 <set_usr_val>:
 b60:	0f 1f 44 00 00       	nopl   0x0(%rax,%rax,1)
 b65:	48 b8 ef cd ab 89 67 	movabs $0x123456789abcdef,%rax
 b6c:	45 23 01 
 b6f:	48 39 c7             	cmp    %rax,%rsi
 b72:	48 0f 47 f8          	cmova  %rax,%rsi
 b76:	90                   	nop    // STAC	
 b77:	90                   	nop
 b78:	90                   	nop
 b79:	31 c0                	xor    %eax,%eax
 b7b:	89 37                	mov    %edi,(%rsi)
 b7d:	b0 01                	mov    $0x1,%al
 b7f:	90                   	nop    // scope_fault: CLAC
 b80:	90                   	nop
 b81:	90                   	nop
 b82:	2e e9 00 00 00 00    	cs jmp b88 <set_usr_val+0x28>

GCC 14 and 15 are not so smart and create an extra error exit for it:

0000000000000bd0 <set_usr_val>:
 bd0:	e8 00 00 00 00       	call   bd5 <set_usr_val+0x5>
 bd5:	48 b8 ef cd ab 89 67 	movabs $0x123456789abcdef,%rax
 bdc:	45 23 01 
 bdf:	48 39 c6             	cmp    %rax,%rsi
 be2:	48 0f 47 f0          	cmova  %rax,%rsi
 be6:	90                   	nop    // STAC
 be7:	90                   	nop
 be8:	90                   	nop
 be9:	89 3e                	mov    %edi,(%rsi)
 beb:	90                   	nop    // CLAC
 bec:	90                   	nop
 bed:	90                   	nop
 bee:	b8 01 00 00 00       	mov    $0x1,%eax
 bf3:	e9 00 00 00 00       	jmp    bf8 <set_usr_val+0x28>
 bf8:	90                   	nop    // scope_fault: CLAC
 bf9:	90                   	nop
 bfa:	90                   	nop
 bfb:	31 c0                	xor    %eax,%eax
 bfd:	e9 00 00 00 00       	jmp    c02 <set_usr_val+0x32>


That said, the series implements the scope infrastructure and converts the
existing users in futex, x86/futex and select over to the new scheme. So
far it nicely held up in testing.

The series applies on top of Linus tree and is also available from git:

    git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git uaccess/masked

Changes vs. V1:
	- use scopes with automatic cleanup
	- provide read/write/rw variants to accommodate PowerPC
	- use the proper rw variant in the futex code
	- avoid the read/write begin/end mismatch by implementation :)
	- implement u64 user access for some shady ARM variant which lacks it

Thanks,

	tglx
---
Thomas Gleixner (6):
      ARM: uaccess: Implement missing __get_user_asm_dword()
      kbuild: Disable asm goto on clang < 17
      uaccess: Provide scoped masked user access regions
      futex: Convert to scoped masked user access
      x86/futex: Convert to scoped masked user access
      select: Convert to scoped masked user access

---
 arch/arm/include/asm/uaccess.h |   17 ++++
 arch/x86/include/asm/futex.h   |   76 ++++++++------------
 fs/select.c                    |   14 +--
 include/linux/uaccess.h        |  151 +++++++++++++++++++++++++++++++++++++++++
 init/Kconfig                   |    7 +
 kernel/futex/futex.h           |   37 +---------
 6 files changed, 214 insertions(+), 88 deletions(-)

[patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Thomas Gleixner]

When CONFIG_CPU_SPECTRE=n then get_user() is missing the 8 byte ASM variant
for no real good reason. This prevents using get_user(u64) in generic code.

Implement it as a sequence of two 4-byte reads with LE/BE awareness.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Closes: https://lore.kernel.org/oe-kbuild-all/202509120155.pFgwfeUD-lkp@intel.com/
---
V2: New patch to fix the 0-day fallout
---
 arch/arm/include/asm/uaccess.h |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -295,6 +295,7 @@ do {									\
 	case 1:	__get_user_asm_byte(__gu_val, __gu_addr, err, __t); break;	\
 	case 2:	__get_user_asm_half(__gu_val, __gu_addr, err, __t); break;	\
 	case 4:	__get_user_asm_word(__gu_val, __gu_addr, err, __t); break;	\
+	case 8:	__get_user_asm_dword(__gu_val, __gu_addr, err, __t); break;	\
 	default: (__gu_val) = __get_user_bad();				\
 	}								\
 	uaccess_restore(__ua_flags);					\
@@ -353,6 +354,22 @@ do {									\
 #define __get_user_asm_word(x, addr, err, __t)			\
 	__get_user_asm(x, addr, err, "ldr" __t)
 
+#ifdef __ARMEB__
+#define __WORD0_OFFS	4
+#define __WORD1_OFFS	0
+#else
+#define __WORD0_OFFS	0
+#define __WORD1_OFFS	4
+#endif
+
+#define __get_user_asm_dword(x, addr, err, __t)				\
+	({								\
+	unsigned long __w0, __w1;					\
+	__get_user_asm(__w0, addr + __WORD0_OFFS, err, "ldr" __t);	\
+	__get_user_asm(__w1, addr + __WORD1_OFFS, err, "ldr" __t);	\
+	(x) = ((u64)__w1 << 32) | (u64) __w0;				\
+})
+
 #define __put_user_switch(x, ptr, __err, __fn)				\
 	do {								\
 		const __typeof__(*(ptr)) __user *__pu_ptr = (ptr);	\

[patch V2 2/6] kbuild: Disable asm goto on clang < 17

[Thomas Gleixner]

clang < 17 fails to use scope local labels with asm goto:

     {
     	__label__ local_lbl;
	...
	unsafe_get_user(uval, uaddr, local_lbl);
	...
	return 0;
	local_lbl:
		return -EFAULT;
     }

when two such scopes exist in the same function:

  error: cannot jump from this asm goto statement to one of its possible targets

That prevents using local labels for a cleanup based user access mechanism.

As there is no way to provide a simple test case for the 'depends on' test
in Kconfig, mark ASM goto broken on clang versions < 17 to get this road
block out of the way.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Nathan Chancellor <nathan@kernel.org>
---
V2: New patch
---
 init/Kconfig |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

--- a/init/Kconfig
+++ b/init/Kconfig
@@ -96,9 +96,14 @@ config GCC_ASM_GOTO_OUTPUT_BROKEN
 	default y if GCC_VERSION >= 120000 && GCC_VERSION < 120400
 	default y if GCC_VERSION >= 130000 && GCC_VERSION < 130300
 
+config CLANG_ASM_GOTO_OUTPUT_BROKEN
+	bool
+	depends on CC_IS_CLANG
+	default y if CLANG_VERSION < 170000
+
 config CC_HAS_ASM_GOTO_OUTPUT
 	def_bool y
-	depends on !GCC_ASM_GOTO_OUTPUT_BROKEN
+	depends on !GCC_ASM_GOTO_OUTPUT_BROKEN && !CLANG_ASM_GOTO_OUTPUT_BROKEN
 	depends on $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)
 
 config CC_HAS_ASM_GOTO_TIED_OUTPUT

[patch V2 3/6] uaccess: Provide scoped masked user access regions

[Thomas Gleixner]

User space access regions are tedious and require similar code patterns all
over the place:

     	if (!user_read_access_begin(from, sizeof(*from)))
		return -EFAULT;
	unsafe_get_user(val, from, Efault);
	user_read_access_end();
	return 0;
Efault:
	user_read_access_end();
	return -EFAULT;

This got worse with the recend addition of masked user access, which
optimizes the speculation prevention:

	if (can_do_masked_user_access())
		from = masked_user_read_access_begin((from));
	else if (!user_read_access_begin(from, sizeof(*from)))
		return -EFAULT;
	unsafe_get_user(val, from, Efault);
	user_read_access_end();
	return 0;
Efault:
	user_read_access_end();
	return -EFAULT;

There have been issues with using the wrong user_*_access_end() variant in
the error path and other typical Copy&Pasta problems, e.g. using the wrong
fault label in the user accessor which ends up using the wrong accesss end
variant. 

These patterns beg for scopes with automatic cleanup. The resulting outcome
is:
    	scoped_masked_user_read_access(from, return -EFAULT,
		scoped_get_user(val, from); );
	return 0;

The scope guarantees the proper cleanup for the access mode is invoked both
in the success and the failure (fault) path

The fault label is scope local and always 'scope_fault:', which prevents
mixing up fault labels between scopes. It is marked __maybe_unused so that
user access, which does not use a fault label and modifies a result
variable in the exception handler, does not cause a defined but unused
warning.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
---
V2: Remove the shady wrappers around the opening and use scopes with automatic cleanup
---
 include/linux/uaccess.h |  151 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 151 insertions(+)

--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -2,6 +2,7 @@
 #ifndef __LINUX_UACCESS_H__
 #define __LINUX_UACCESS_H__
 
+#include <linux/cleanup.h>
 #include <linux/fault-inject-usercopy.h>
 #include <linux/instrumented.h>
 #include <linux/minmax.h>
@@ -35,9 +36,17 @@
 
 #ifdef masked_user_access_begin
  #define can_do_masked_user_access() 1
+# ifndef masked_user_write_access_begin
+#  define masked_user_write_access_begin masked_user_access_begin
+# endif
+# ifndef masked_user_read_access_begin
+#  define masked_user_read_access_begin masked_user_access_begin
+#endif
 #else
  #define can_do_masked_user_access() 0
  #define masked_user_access_begin(src) NULL
+ #define masked_user_read_access_begin(src) NULL
+ #define masked_user_write_access_begin(src) NULL
  #define mask_user_address(src) (src)
 #endif
 
@@ -569,6 +578,148 @@ static inline void user_access_restore(u
 #define user_read_access_end user_access_end
 #endif
 
+/* Define RW variant so the below _mode macro expansion works */
+#define masked_user_rw_access_begin(u)	masked_user_access_begin(u)
+#define user_rw_access_begin(u, s)	user_access_begin(u, s)
+#define user_rw_access_end()		user_access_end()
+
+/* Scoped user access */
+#define USER_ACCESS_GUARD(_mode)					\
+static __always_inline void __user *					\
+class_masked_user_##_mode##_begin(void __user *ptr)			\
+{									\
+	return ptr;							\
+}									\
+									\
+static __always_inline void						\
+class_masked_user_##_mode##_end(void __user *ptr)			\
+{									\
+	user_##_mode##_access_end();					\
+}									\
+									\
+DEFINE_CLASS(masked_user_ ##_mode## _access, void __user *,		\
+	     class_masked_user_##_mode##_end(_T),			\
+	     class_masked_user_##_mode##_begin(ptr), void __user *ptr)	\
+									\
+static __always_inline class_masked_user_##_mode##_access_t		\
+class_masked_user_##_mode##_access_ptr(void __user *scope)		\
+{									\
+	return scope;							\
+}									\
+
+USER_ACCESS_GUARD(read)
+USER_ACCESS_GUARD(write)
+USER_ACCESS_GUARD(rw)
+#undef USER_ACCESS_GUARD
+
+/**
+ * __scoped_masked_user_access - Open a scope for masked user access
+ * @_mode:	The mode of the access class (read, write, rw)
+ * @_uptr:	The pointer to access user space memory
+ * @_ecode:	Code to inject for the failure case
+ * @_code:	The code to inject inside the scope
+ *
+ * When the scope is left user_##@_mode##_access_end() is invoked, if the
+ * corresponding user_##@_mode##_masked_begin() succeeded.
+ *
+ * The user access function inside the scope must use a fault label, which
+ * is inside the scope. __scoped_masked_user_access() provides a default
+ * label @scope_label, which is placed right before @_ecode. This label is
+ * scope local, so multiple masked user scopes can be in one function.
+ *
+ * The user access helpers scoped_put_user() and scoped_get_user() use
+ * @scope_label automatically.
+ *
+ *  A single line statement in the scope::
+ *
+ *	scoped_masked_user_read_access(ptr, return false,
+ *		scoped_get_user(val, ptr););
+ *
+ *  Multi-line statement::
+ *
+ *	scoped_masked_user_rw_access(ptr, return false, {
+ *		scoped_get_user(rval, &ptr->rval);
+ *		scoped_put_user(wval, &ptr->wval);
+ *	});
+ */
+#define __scoped_masked_user_access(_mode, _uptr, _ecode, _code)	\
+do {									\
+	unsigned long size = sizeof(*(_uptr));				\
+	typeof((_uptr)) _tmpptr = (_uptr);				\
+	bool proceed = true;						\
+									\
+	/*								\
+	 * Must be outside the CLASS scope below to handle the fail	\
+	 * of the non-masked access_begin() case correctly.		\
+	 */								\
+	if (can_do_masked_user_access())				\
+		_tmpptr = masked_user_##_mode##_access_begin((_uptr));	\
+	else								\
+		proceed = user_##_mode##_access_begin((_uptr), size);	\
+									\
+	if (!proceed) {							\
+		_ecode;							\
+	} else {							\
+		__label__ scope_fault;					\
+		CLASS(masked_user_##_mode##_access, scope) (_tmpptr);	\
+		/* Force modified pointer usage in @_code */		\
+		const typeof((_uptr)) _uptr = _tmpptr;			\
+									\
+		_code;							\
+		if (0) {						\
+		scope_fault:						\
+			__maybe_unused;					\
+			_ecode;						\
+		}							\
+	}								\
+} while (0)
+
+/**
+ * scoped_masked_user_read_access - Start a scoped __user_masked_read_access()
+ * @_usrc:	Pointer to the user space address to read from
+ * @_ecode:	Code to inject for the failure case
+ *
+ * For further information see __scoped_masked_user_access() above.
+ */
+#define scoped_masked_user_read_access(_usrc, _ecode, _code)	\
+	__scoped_masked_user_access(read, (_usrc), _ecode, ({_code}))
+
+/**
+ * scoped_masked_user_write_access - Start a scoped __user_masked_write_access()
+ * @_udst:	Pointer to the user space address to write to
+ * @_ecode:	Code to inject for the failure case
+ *
+ * For further information see __scoped_masked_user_access() above.
+ */
+#define scoped_masked_user_write_access(_udst, _ecode, _code)	\
+	__scoped_masked_user_access(write, (_udst), _ecode, ({_code}))
+
+/**
+ * scoped_masked_user_rw_access - Start a scoped __user_masked_rw_access()
+ * @_uptr	Pointer to the user space address to read from and write to
+ * @_ecode:	Code to inject for the failure case
+ *
+ * For further information see __scoped_masked_user_access() above.
+ */
+#define scoped_masked_user_rw_access(_uptr, _ecode, _code)	\
+	__scoped_masked_user_access(rw, (_uptr), _ecode, ({_code}))
+
+/**
+ * scoped_get_user - Read user memory from within a scoped masked access section
+ * @_dst:	The destination variable to store the read value
+ * @_usrc:	Pointer to the user space address to read from
+ */
+#define scoped_get_user(_dst, _usrc)				\
+	unsafe_get_user((_dst), (_usrc), scope_fault)
+
+/**
+ * scoped_put_user - Write user memory from within a scoped masked access section
+ * @_val:	Value to write
+ * @_udst:	Pointer to the user space address to write to
+ */
+#define scoped_put_user(_val, _udst)				\
+	unsafe_put_user((_val), (_udst), scope_fault)
+
 #ifdef CONFIG_HARDENED_USERCOPY
 void __noreturn usercopy_abort(const char *name, const char *detail,
 			       bool to_user, unsigned long offset,

[patch V2 4/6] futex: Convert to scoped masked user access

[Thomas Gleixner]

From: Thomas Gleixner <tglx@linutronix.de>

Replace the open coded implementation with the scoped masked user access
mechanism.

No functional change intended

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: "André Almeida" <andrealmeid@igalia.com>
---
V2: Convert to scoped variant
---
 kernel/futex/futex.h |   37 ++++---------------------------------
 1 file changed, 4 insertions(+), 33 deletions(-)
---
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -285,48 +285,19 @@ static inline int futex_cmpxchg_value_lo
  * This does a plain atomic user space read, and the user pointer has
  * already been verified earlier by get_futex_key() to be both aligned
  * and actually in user space, just like futex_atomic_cmpxchg_inatomic().
- *
- * We still want to avoid any speculation, and while __get_user() is
- * the traditional model for this, it's actually slower than doing
- * this manually these days.
- *
- * We could just have a per-architecture special function for it,
- * the same way we do futex_atomic_cmpxchg_inatomic(), but rather
- * than force everybody to do that, write it out long-hand using
- * the low-level user-access infrastructure.
- *
- * This looks a bit overkill, but generally just results in a couple
- * of instructions.
  */
 static __always_inline int futex_get_value(u32 *dest, u32 __user *from)
 {
-	u32 val;
-
-	if (can_do_masked_user_access())
-		from = masked_user_access_begin(from);
-	else if (!user_read_access_begin(from, sizeof(*from)))
-		return -EFAULT;
-	unsafe_get_user(val, from, Efault);
-	user_read_access_end();
-	*dest = val;
+	scoped_masked_user_read_access(from, return -EFAULT,
+	       scoped_get_user(*dest, from); );
 	return 0;
-Efault:
-	user_read_access_end();
-	return -EFAULT;
 }
 
 static __always_inline int futex_put_value(u32 val, u32 __user *to)
 {
-	if (can_do_masked_user_access())
-		to = masked_user_access_begin(to);
-	else if (!user_write_access_begin(to, sizeof(*to)))
-		return -EFAULT;
-	unsafe_put_user(val, to, Efault);
-	user_write_access_end();
+	scoped_masked_user_write_access(to, return -EFAULT,
+	       scoped_put_user(val, to); );
 	return 0;
-Efault:
-	user_write_access_end();
-	return -EFAULT;
 }
 
 static inline int futex_get_value_locked(u32 *dest, u32 __user *from)

[patch V2 5/6] x86/futex: Convert to scoped masked user access

[Thomas Gleixner]

Replace the open coded implementation with the scoped masked user access
mechanism.

No functional change intended.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: x86@kernel.org
---
V2:	Convert to scoped masked access
	Use RW access functions - Christophe
---
 arch/x86/include/asm/futex.h |   76 ++++++++++++++++++-------------------------
 1 file changed, 32 insertions(+), 44 deletions(-)
---
--- a/arch/x86/include/asm/futex.h
+++ b/arch/x86/include/asm/futex.h
@@ -48,37 +48,29 @@ do {								\
 static __always_inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
 		u32 __user *uaddr)
 {
-	if (can_do_masked_user_access())
-		uaddr = masked_user_access_begin(uaddr);
-	else if (!user_access_begin(uaddr, sizeof(u32)))
-		return -EFAULT;
-
-	switch (op) {
-	case FUTEX_OP_SET:
-		unsafe_atomic_op1("xchgl %0, %2", oval, uaddr, oparg, Efault);
-		break;
-	case FUTEX_OP_ADD:
-		unsafe_atomic_op1(LOCK_PREFIX "xaddl %0, %2", oval,
-				   uaddr, oparg, Efault);
-		break;
-	case FUTEX_OP_OR:
-		unsafe_atomic_op2("orl %4, %3", oval, uaddr, oparg, Efault);
-		break;
-	case FUTEX_OP_ANDN:
-		unsafe_atomic_op2("andl %4, %3", oval, uaddr, ~oparg, Efault);
-		break;
-	case FUTEX_OP_XOR:
-		unsafe_atomic_op2("xorl %4, %3", oval, uaddr, oparg, Efault);
-		break;
-	default:
-		user_access_end();
-		return -ENOSYS;
-	}
-	user_access_end();
+	scoped_masked_user_rw_access(uaddr, return -EFAULT, {
+		switch (op) {
+		case FUTEX_OP_SET:
+			unsafe_atomic_op1("xchgl %0, %2", oval, uaddr, oparg, scope_fault);
+			break;
+		case FUTEX_OP_ADD:
+			unsafe_atomic_op1(LOCK_PREFIX "xaddl %0, %2", oval,
+					  uaddr, oparg, scope_fault);
+			break;
+		case FUTEX_OP_OR:
+			unsafe_atomic_op2("orl %4, %3", oval, uaddr, oparg, scope_fault);
+			break;
+		case FUTEX_OP_ANDN:
+			unsafe_atomic_op2("andl %4, %3", oval, uaddr, ~oparg, scope_fault);
+			break;
+		case FUTEX_OP_XOR:
+			unsafe_atomic_op2("xorl %4, %3", oval, uaddr, oparg, scope_fault);
+			break;
+		default:
+			return -ENOSYS;
+		}
+	});
 	return 0;
-Efault:
-	user_access_end();
-	return -EFAULT;
 }
 
 static inline int futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
@@ -86,20 +78,16 @@ static inline int futex_atomic_cmpxchg_i
 {
 	int ret = 0;
 
-	if (can_do_masked_user_access())
-		uaddr = masked_user_access_begin(uaddr);
-	else if (!user_access_begin(uaddr, sizeof(u32)))
-		return -EFAULT;
-	asm volatile("\n"
-		"1:\t" LOCK_PREFIX "cmpxchgl %3, %2\n"
-		"2:\n"
-		_ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG, %0) \
-		: "+r" (ret), "=a" (oldval), "+m" (*uaddr)
-		: "r" (newval), "1" (oldval)
-		: "memory"
-	);
-	user_access_end();
-	*uval = oldval;
+	scoped_masked_user_rw_access(uaddr, return -EFAULT, {
+		asm volatile("\n"
+			     "1:\t" LOCK_PREFIX "cmpxchgl %3, %2\n"
+			     "2:\n"
+			     _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG, %0) \
+			     : "+r" (ret), "=a" (oldval), "+m" (*uaddr)
+			     : "r" (newval), "1" (oldval)
+			     : "memory");
+		*uval = oldval;
+	});
 	return ret;
 }
 

[patch V2 6/6] select: Convert to scoped masked user access

[Thomas Gleixner]

From: Thomas Gleixner <tglx@linutronix.de>

Replace the open coded implementation with the scoped masked user access
mechanism.

No functional change intended.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: linux-fsdevel@vger.kernel.org
---
 fs/select.c |   14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)
---
--- a/fs/select.c
+++ b/fs/select.c
@@ -776,18 +776,12 @@ static inline int get_sigset_argpack(str
 {
 	// the path is hot enough for overhead of copy_from_user() to matter
 	if (from) {
-		if (can_do_masked_user_access())
-			from = masked_user_access_begin(from);
-		else if (!user_read_access_begin(from, sizeof(*from)))
-			return -EFAULT;
-		unsafe_get_user(to->p, &from->p, Efault);
-		unsafe_get_user(to->size, &from->size, Efault);
-		user_read_access_end();
+		scoped_masked_user_rw_access(from, return -EFAULT, {
+			scoped_get_user(to->p, &from->p);
+			scoped_get_user(to->size, &from->size);
+		});
 	}
 	return 0;
-Efault:
-	user_read_access_end();
-	return -EFAULT;
 }
 
 SYSCALL_DEFINE6(pselect6, int, n, fd_set __user *, inp, fd_set __user *, outp,

Re: [patch V2 2/6] kbuild: Disable asm goto on clang < 17

[Nathan Chancellor]

Hi Thomas,

First of all, sorry you got bit by this issue.

On Tue, Sep 16, 2025 at 06:33:11PM +0200, Thomas Gleixner wrote:
> clang < 17 fails to use scope local labels with asm goto:
> 
>      {
>      	__label__ local_lbl;
> 	...
> 	unsafe_get_user(uval, uaddr, local_lbl);
> 	...
> 	return 0;
> 	local_lbl:
> 		return -EFAULT;
>      }
> 
> when two such scopes exist in the same function:
> 
>   error: cannot jump from this asm goto statement to one of its possible targets

For the record, this is not specific to local labels, unique function
labels could trigger this error as well, as demonstrated by Nick's test
case:

https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1636342477

> That prevents using local labels for a cleanup based user access mechanism.

Indeed. This has only popped up a couple of times in the past couple of
years and each time it has been easy enough to work around by shuffling
the use of asm goto but as cleanup gets used in more places, this is
likely to cause problems.

> As there is no way to provide a simple test case for the 'depends on' test
> in Kconfig, mark ASM goto broken on clang versions < 17 to get this road
> block out of the way.

That being said, the commit title and message always references asm goto
in the general sense but this change only affects asm goto with outputs.
Is it sufficient to resolve the issues you were seeing? As far as I
understand it, the general issue can affect asm goto with or without
outputs but I assume x86 won't have any issues because the label is not
used in __get_user_asm when asm goto with outputs is not supported?

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Nathan Chancellor <nathan@kernel.org>
> ---
> V2: New patch
> ---
>  init/Kconfig |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -96,9 +96,14 @@ config GCC_ASM_GOTO_OUTPUT_BROKEN
>  	default y if GCC_VERSION >= 120000 && GCC_VERSION < 120400
>  	default y if GCC_VERSION >= 130000 && GCC_VERSION < 130300
>  
> +config CLANG_ASM_GOTO_OUTPUT_BROKEN
> +	bool
> +	depends on CC_IS_CLANG
> +	default y if CLANG_VERSION < 170000

Assuming this change sticks, please consider including links to the
original bug report and the fix in LLVM:

  https://github.com/ClangBuiltLinux/linux/issues/1886
  https://github.com/llvm/llvm-project/commit/f023f5cdb2e6c19026f04a15b5a935c041835d14

> +
>  config CC_HAS_ASM_GOTO_OUTPUT
>  	def_bool y
> -	depends on !GCC_ASM_GOTO_OUTPUT_BROKEN
> +	depends on !GCC_ASM_GOTO_OUTPUT_BROKEN && !CLANG_ASM_GOTO_OUTPUT_BROKEN
>  	depends on $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)
>  
>  config CC_HAS_ASM_GOTO_TIED_OUTPUT
> 

Re: [patch V2 2/6] kbuild: Disable asm goto on clang < 17

[Thomas Gleixner]

Nathan!

On Tue, Sep 16 2025 at 11:44, Nathan Chancellor wrote:
> First of all, sorry you got bit by this issue.

The real annoying thing was that I could not makes sense of the error
messages and when I started shuffling code around for analysis it got
worse by failing reliably even with one instance or it exposed random
other incomprehensible errors which did not help analysis either.

Sh*t happens :)

> On Tue, Sep 16, 2025 at 06:33:11PM +0200, Thomas Gleixner wrote:
>> clang < 17 fails to use scope local labels with asm goto:
>> 
>>      {
>>      	__label__ local_lbl;
>> 	...
>> 	unsafe_get_user(uval, uaddr, local_lbl);
>> 	...
>> 	return 0;
>> 	local_lbl:
>> 		return -EFAULT;
>>      }
>> 
>> when two such scopes exist in the same function:
>> 
>>   error: cannot jump from this asm goto statement to one of its possible targets
>
> For the record, this is not specific to local labels, unique function
> labels could trigger this error as well, as demonstrated by Nick's test
> case:
>
> https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1636342477

Ah! I somehow failed to find this one.

I was actually trying to create a simple reproducer for using in the
depends on $(success,echo...) magic and could not manage.

The test case in the issue tracker is really helpful as it can be
condensed into the obfuscated C-code contest format required for
'depends on' checks. So we don't need the version number hack for
detecting it. That's definitely preferred as it catches potential
misbehaviour of later versions and of other compilers as well.

I'll send out a revised patch to that effect later.

>> That prevents using local labels for a cleanup based user access mechanism.
>
> Indeed. This has only popped up a couple of times in the past couple of
> years and each time it has been easy enough to work around by shuffling
> the use of asm goto but as cleanup gets used in more places, this is
> likely to cause problems.

Yes. I noticed that moving the label around or rearraning code slightly
makes it go away or even worse, but that's not a real solution :)

>> As there is no way to provide a simple test case for the 'depends on' test
>> in Kconfig, mark ASM goto broken on clang versions < 17 to get this road
>> block out of the way.
>
> That being said, the commit title and message always references asm goto
> in the general sense but this change only affects asm goto with
> outputs.

Right, that's misleading.

> Is it sufficient to resolve the issues you were seeing? As far as I
> understand it, the general issue can affect asm goto with or without
> outputs but I assume x86 won't have any issues because the label is not
> used in __get_user_asm when asm goto with outputs is not supported?

I haven't seen a problem with that yet. So yes, as things stand that
seems to be a ASM_GOTO_OUTPUT issue.

>> +config CLANG_ASM_GOTO_OUTPUT_BROKEN
>> +	bool
>> +	depends on CC_IS_CLANG
>> +	default y if CLANG_VERSION < 170000
>
> Assuming this change sticks, please consider including links to the
> original bug report and the fix in LLVM:
>
>   https://github.com/ClangBuiltLinux/linux/issues/1886
>   https://github.com/llvm/llvm-project/commit/f023f5cdb2e6c19026f04a15b5a935c041835d14

Sure! That's indeed useful.

Thanks,

        tglx

[patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17

[Thomas Gleixner]

clang < 17 fails to use scope local labels with CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y:

     {
     	__label__ local_lbl;
	...
	unsafe_get_user(uval, uaddr, local_lbl);
	...
	return 0;
	local_lbl:
		return -EFAULT;
     }

when two such scopes exist in the same function:

  error: cannot jump from this asm goto statement to one of its possible targets

There are other failure scenarios. Shuffling code around slightly makes it
worse and fail even with one instance.

That issue prevents using local labels for a cleanup based user access
mechanism.

After failed attempts to provide a simple enough test case for the 'depends
on' test in Kconfig, the initial cure was to mark ASM goto broken on clang
versions < 17 to get this road block out of the way.

But Nathan pointed out that this is a known clang issue and indeed affects
clang < version 17 in combination with cleanup(). It's not even required to
use local labels for that.

The clang issue tracker has a small enough test case, which can be used as
a test in the 'depends on' section of CC_HAS_ASM_GOTO_OUTPUT:

void bar(void **);
void* baz();

int  foo (void) {
    {
	    asm goto("jmp %l0"::::l0);
	    return 0;
l0:
	    return 1;
    }
    void *x __attribute__((cleanup(bar))) = baz();
    {
	    asm goto("jmp %l0"::::l1);
	    return 42;
l1:
	    return 0xff;
    }
}

Add another dependency to config CC_HAS_ASM_GOTO_OUTPUT for it and use the
clang issue tracker test case for detection by condensing it to obfuscated
C-code contest format. This reliably catches the problem on clang < 17 and
did not show any issues on the non known to be broken GCC versions.

That test might be sufficient to catch all issues and therefore could
replace the existing test, but keeping that around does no harm either.

Thanks to Nathan for pointing to the relevant clang issue!

Suggested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Nathan Chancellor <nathan@kernel.org>
Link: https://github.com/ClangBuiltLinux/linux/issues/1886
Link: https://github.com/llvm/llvm-project/commit/f023f5cdb2e6c19026f04a15b5a935c041835d14
---
V2a: Use the reproducer from llvm
V2: New patch
---
 init/Kconfig |    3 +++
 1 file changed, 3 insertions(+)

--- a/init/Kconfig
+++ b/init/Kconfig
@@ -99,7 +99,10 @@ config GCC_ASM_GOTO_OUTPUT_BROKEN
 config CC_HAS_ASM_GOTO_OUTPUT
 	def_bool y
 	depends on !GCC_ASM_GOTO_OUTPUT_BROKEN
+	# Find basic issues
 	depends on $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)
+	# Detect buggy clang, fixed in clang-17
+	depends on $(success,echo 'void b(void **);void* c();int f(void){{asm goto("jmp %l0"::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto("jmp %l0"::::l1);return 2;l1:return 1;}}' | $(CC) -x c - -c -o /dev/null)
 
 config CC_HAS_ASM_GOTO_TIED_OUTPUT
 	depends on CC_HAS_ASM_GOTO_OUTPUT

Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Russell King (Oracle)]

On Tue, Sep 16, 2025 at 06:33:09PM +0200, Thomas Gleixner wrote:
> When CONFIG_CPU_SPECTRE=n then get_user() is missing the 8 byte ASM variant
> for no real good reason. This prevents using get_user(u64) in generic code.

I'm sure you will eventually discover the reason when you start getting
all the kernel build bot warnings that will result from a cast from a
u64 to a pointer.

You're not the first to try implementing this.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17

[Nathan Chancellor]

On Tue, Sep 16, 2025 at 10:56:36PM +0200, Thomas Gleixner wrote:
> clang < 17 fails to use scope local labels with CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y:
> 
>      {
>      	__label__ local_lbl;
> 	...
> 	unsafe_get_user(uval, uaddr, local_lbl);
> 	...
> 	return 0;
> 	local_lbl:
> 		return -EFAULT;
>      }
> 
> when two such scopes exist in the same function:
> 
>   error: cannot jump from this asm goto statement to one of its possible targets
> 
> There are other failure scenarios. Shuffling code around slightly makes it
> worse and fail even with one instance.
> 
> That issue prevents using local labels for a cleanup based user access
> mechanism.
> 
> After failed attempts to provide a simple enough test case for the 'depends
> on' test in Kconfig, the initial cure was to mark ASM goto broken on clang
> versions < 17 to get this road block out of the way.
> 
> But Nathan pointed out that this is a known clang issue and indeed affects
> clang < version 17 in combination with cleanup(). It's not even required to
> use local labels for that.
> 
> The clang issue tracker has a small enough test case, which can be used as
> a test in the 'depends on' section of CC_HAS_ASM_GOTO_OUTPUT:
> 
> void bar(void **);
> void* baz();

I would recommend

  void* baz(void);

here and in the actual test to preemptively harden against the
possibility of a future where -Wstrict-prototypes is turned on as an
error by default (as unlikely as this may be, it has been brought up
before [1]), as I would not want this to get silently disabled.

> int  foo (void) {
>     {
> 	    asm goto("jmp %l0"::::l0);
> 	    return 0;
> l0:
> 	    return 1;
>     }
>     void *x __attribute__((cleanup(bar))) = baz();
>     {
> 	    asm goto("jmp %l0"::::l1);
> 	    return 42;
> l1:
> 	    return 0xff;
>     }
> }
> 
> Add another dependency to config CC_HAS_ASM_GOTO_OUTPUT for it and use the
> clang issue tracker test case for detection by condensing it to obfuscated
> C-code contest format. This reliably catches the problem on clang < 17 and
> did not show any issues on the non known to be broken GCC versions.
> 
> That test might be sufficient to catch all issues and therefore could
> replace the existing test, but keeping that around does no harm either.
> 
> Thanks to Nathan for pointing to the relevant clang issue!
> 
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1886
> Link: https://github.com/llvm/llvm-project/commit/f023f5cdb2e6c19026f04a15b5a935c041835d14

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> V2a: Use the reproducer from llvm
> V2: New patch
> ---
>  init/Kconfig |    3 +++
>  1 file changed, 3 insertions(+)
> 
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -99,7 +99,10 @@ config GCC_ASM_GOTO_OUTPUT_BROKEN
>  config CC_HAS_ASM_GOTO_OUTPUT
>  	def_bool y
>  	depends on !GCC_ASM_GOTO_OUTPUT_BROKEN
> +	# Find basic issues

Maybe "Detect basic support" or something like that? This is not really
an "issues" test, more of a "does the compiler support it at all?" test
if I understand correctly.

>  	depends on $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)
> +	# Detect buggy clang, fixed in clang-17
> +	depends on $(success,echo 'void b(void **);void* c();int f(void){{asm goto("jmp %l0"::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto("jmp %l0"::::l1);return 2;l1:return 1;}}' | $(CC) -x c - -c -o /dev/null)
>  
>  config CC_HAS_ASM_GOTO_TIED_OUTPUT
>  	depends on CC_HAS_ASM_GOTO_OUTPUT

Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Thomas Gleixner]

On Tue, Sep 16 2025 at 22:26, Russell King wrote:
> On Tue, Sep 16, 2025 at 06:33:09PM +0200, Thomas Gleixner wrote:
>> When CONFIG_CPU_SPECTRE=n then get_user() is missing the 8 byte ASM variant
>> for no real good reason. This prevents using get_user(u64) in generic code.
>
> I'm sure you will eventually discover the reason when you start getting
> all the kernel build bot warnings that will result from a cast from a
> u64 to a pointer.

I really don't know which cast you are talking about.

  	u64 __user *uaddr =  ...;
	u64 val;

        ....
        unsafe_get_user(val, uaddr, fault);

The only casts in this macro maze are in __get_user_err():

 1) Casting the uaddr pointer to unsigned long:

     unsigned long __gu_addr = (unsigned long)(ptr);

    which is correct because a *u64 pointer is still only 32bit wide on a
    32bit machine, no?

 2) Casting the result:

    (x) = (__typeof__(*(ptr)))__gu_val;

    which is casting to the type to which the pointer points to,
    i.e. u64 in this case.

I definitely checked the ASM result after I successfully compiled the
above w/o warnings. It compiles to:

 ad0:	ee032f10 	mcr	15, 0, r2, cr3, cr0, {0}
 ad4:	e3a00000 	mov	r0, #0
 ad8:	e4b3e000 	ldrt	lr, [r3], #0
 adc:	e2833004 	add	r3, r3, #4
 ae0:	e4b32000 	ldrt	r2, [r3], #0
 ae4:	ee03cf10 	mcr	15, 0, ip, cr3, cr0, {0}
 ae8:	e16f0f10 	clz	r0, r0
 aec:	e581e000 	str	lr, [r1]
 af0:	e5812004 	str	r2, [r1, #4]

which is magically correct despite the fact that I missed to change the
type of __gu_val to 'unsigned long long'. I just noticed when I tried to
figure out which cast you were referring to.

The wonderful and surprising world of macro preprocessing. :)

That unsigned long long is not hurtful as the compiler is smart enough
to optimize it away when __get_user_err() is invoked to read an u8 from
user:

 b18:	ee033f10 	mcr	15, 0, r3, cr3, cr0, {0}
 b1c:	e3a03000 	mov	r3, #0
 b20:	e4f04000 	ldrbt	r4, [r0], #0
 b24:	ee032f10 	mcr	15, 0, r2, cr3, cr0, {0}
 b28:	e16f0f13 	clz	r0, r3
 b2c:	e5c14000 	strb	r4, [r1]

which is exactly the same result as before this change.

Thanks,

        tglx

--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -286,7 +286,7 @@ extern int __put_user_8(void *, unsigned
 #define __get_user_err(x, ptr, err, __t)				\
 do {									\
 	unsigned long __gu_addr = (unsigned long)(ptr);			\
-	unsigned long __gu_val;						\
+	unsigned long long __gu_val;					\
 	unsigned int __ua_flags;					\
 	__chk_user_ptr(ptr);						\
 	might_fault();							\

Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Russell King (Oracle)]

On Wed, Sep 17, 2025 at 07:48:00AM +0200, Thomas Gleixner wrote:
> On Tue, Sep 16 2025 at 22:26, Russell King wrote:
> > On Tue, Sep 16, 2025 at 06:33:09PM +0200, Thomas Gleixner wrote:
> >> When CONFIG_CPU_SPECTRE=n then get_user() is missing the 8 byte ASM variant
> >> for no real good reason. This prevents using get_user(u64) in generic code.
> >
> > I'm sure you will eventually discover the reason when you start getting
> > all the kernel build bot warnings that will result from a cast from a
> > u64 to a pointer.
> 
> I really don't know which cast you are talking about.

I'll grant you that the problem is not obvious. It comes about because
of all the different types that get_user() is subject to - it's not
just integers, it's also pointers.

The next bit to realise is that casting between integers that are not
the same size as a pointer causes warnings. For example, casting
between a 64-bit integer type and pointer type causes the compiler to
emit a warning. It doesn't matter if the code path ends up being
optimised away, the warning is still issued.

Putting together a simple test case, where the only change is making
__gu_val an unsigned long long:

t.c: In function ‘get_ptr’:
t.c:40:15: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   40 |         (x) = (__typeof__(*(ptr)))__gu_val;                             \
      |               ^
t.c:21:9: note: in expansion of macro ‘__get_user_err’
   21 |         __get_user_err((x), (ptr), __gu_err, TUSER());                  \
      |         ^~~~~~~~~~~~~~
t.c:102:16: note: in expansion of macro ‘__get_user’
  102 |         return __get_user(p, ptr);
      |                ^~~~~~~~~~

In order for the code you are modifying to be reachable, you need to
build with CONFIG_CPU_SPECTRE disabled. This is produced by:

int get_ptr(void **ptr)
{
        void *p;

        return __get_user(p, ptr);
}

Now, one idea may be to declare __gu_val as:

	__typeof__(x) __gu_val;

but then we run into:

t.c: In function ‘get_ptr’:
t.c:37:29: error: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
   37 |         default: (__gu_val) = __get_user_bad();                         \
      |                             ^
t.c:21:9: note: in expansion of macro ‘__get_user_err’
   21 |         __get_user_err((x), (ptr), __gu_err, TUSER());                  \
      |         ^~~~~~~~~~~~~~
t.c:102:16: note: in expansion of macro ‘__get_user’
  102 |         return __get_user(p, ptr);
      |                ^~~~~~~~~~

You may think this is easy to solve, just change the last cast to:

	(x) = (__typeof__(*(ptr)))(__typeof__(x))__gu_val;

but that doesn't work either (because in the test case __typeof__(x) is
still a pointer type. You can't cast this down to a 32-bit quantity
because that will knock off the upper 32 bits for the case you're trying
to add.

You may think, why not  move this cast into each switch statement...
there will still be warnings because the cast is still reachable at the
point the compiler evaluates the code for warnings, even though the
optimiser gets rid of it later.

Feel free to try to solve this, but I can assure you that you certainly
are not the first. Several people have already tried.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Christophe Leroy]

Le 17/09/2025 à 11:41, Russell King (Oracle) a écrit :
> You may think this is easy to solve, just change the last cast to:
> 
>          (x) = (__typeof__(*(ptr)))(__typeof__(x))__gu_val;
> 
> but that doesn't work either (because in the test case __typeof__(x) is
> still a pointer type. You can't cast this down to a 32-bit quantity
> because that will knock off the upper 32 bits for the case you're trying
> to add.
> 
> You may think, why not  move this cast into each switch statement...
> there will still be warnings because the cast is still reachable at the
> point the compiler evaluates the code for warnings, even though the
> optimiser gets rid of it later.
> 
> Feel free to try to solve this, but I can assure you that you certainly
> are not the first. Several people have already tried.
> 

No such problem on powerpc/32, maybe because we have defined and use 
macro __long_type(x), see below:

#define __get_user_size_allowed(x, ptr, size, retval)		\
do {								\
	retval = 0;						\
	BUILD_BUG_ON(size > sizeof(x));				\
	switch (size) {						\
	case 1: __get_user_asm(x, (u8 __user *)ptr, retval, "lbz"); break;	\
	case 2: __get_user_asm(x, (u16 __user *)ptr, retval, "lhz"); break;	\
	case 4: __get_user_asm(x, (u32 __user *)ptr, retval, "lwz"); break;	\
	case 8: __get_user_asm2(x, (u64 __user *)ptr, retval);  break;	\
	default: x = 0; BUILD_BUG();				\
	}							\
} while (0)

/*
  * This is a type: either unsigned long, if the argument fits into
  * that type, or otherwise unsigned long long.
  */
#define __long_type(x) \
	__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))

#define __get_user(x, ptr)					\
({								\
	long __gu_err;						\
	__long_type(*(ptr)) __gu_val;				\
	__typeof__(*(ptr)) __user *__gu_addr = (ptr);	\
	__typeof__(sizeof(*(ptr))) __gu_size = sizeof(*(ptr));	\
								\
	might_fault();					\
	allow_read_from_user(__gu_addr, __gu_size);		\
	__get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err);	\
	prevent_read_from_user(__gu_addr, __gu_size);		\
	(x) = (__typeof__(*(ptr)))__gu_val;			\
								\
	__gu_err;						\
})

Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Thomas Gleixner]

On Wed, Sep 17 2025 at 10:41, Russell King wrote:
> On Wed, Sep 17, 2025 at 07:48:00AM +0200, Thomas Gleixner wrote:
>
> Putting together a simple test case, where the only change is making
> __gu_val an unsigned long long:
>
> t.c: In function ‘get_ptr’:
> t.c:40:15: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>    40 |         (x) = (__typeof__(*(ptr)))__gu_val;                             \
>       |               ^
> t.c:21:9: note: in expansion of macro ‘__get_user_err’
>    21 |         __get_user_err((x), (ptr), __gu_err, TUSER());                  \
>       |         ^~~~~~~~~~~~~~
> t.c:102:16: note: in expansion of macro ‘__get_user’
>   102 |         return __get_user(p, ptr);
>       |                ^~~~~~~~~~
>
> In order for the code you are modifying to be reachable, you need to
> build with CONFIG_CPU_SPECTRE disabled. This is produced by:
>
> int get_ptr(void **ptr)
> {
>         void *p;
>
>         return __get_user(p, ptr);
> }

Duh, yes. I hate get_user() and I did not notice, because the
allmodconfig build breaks early due to frame size checks, so I was too
lazy to find that config knob and built only a couple of things and an
artificial test case for u64.

But it actually can be solved solvable by switching the casting to:

    (x) = *(__force __typeof__(*(ptr)) *) &__gu_val;

Not pretty, but after upping the frame size limit it builds an
allmodconfig kernel.

The proper thing to use the corresponding sized values within the case
$SIZE sections i.e.:

	size 1: {
       		u8 __gu_val;
                __get_user_asm_byte(__gu_val, __gu_addr, err, __t);
                (x) = *(__force __typeof__(*(ptr)) *) &__gu_val;
		break;
	}
        ...

See below.

> Feel free to try to solve this, but I can assure you that you certainly
> are not the first. Several people have already tried.

Obviously not hard enough. :)

Thanks,

        tglx
---
Subject: ARM: uaccess: Implement missing __get_user_asm_dword()
From: Thomas Gleixner <tglx@linutronix.de>
Date: Fri, 12 Sep 2025 20:16:11 +0200

When CONFIG_CPU_SPECTRE=n then get_user() is missing the 8 byte ASM variant
for no real good reason. This prevents using get_user(u64) in generic code.

Implement it as a sequence of two 4-byte reads with LE/BE awareness and
fixup the size switch case to make get_user(*ptr, **usrptr) work.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Closes: https://lore.kernel.org/oe-kbuild-all/202509120155.pFgwfeUD-lkp@intel.com/
---
V2a: Solve the *ptr issue vs. unsigned long long - Russell
V2: New patch to fix the 0-day fallout
---
 arch/arm/include/asm/uaccess.h |   50 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 44 insertions(+), 6 deletions(-)

--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -286,19 +286,41 @@ extern int __put_user_8(void *, unsigned
 #define __get_user_err(x, ptr, err, __t)				\
 do {									\
 	unsigned long __gu_addr = (unsigned long)(ptr);			\
-	unsigned long __gu_val;						\
 	unsigned int __ua_flags;					\
 	__chk_user_ptr(ptr);						\
 	might_fault();							\
 	__ua_flags = uaccess_save_and_enable();				\
 	switch (sizeof(*(ptr))) {					\
-	case 1:	__get_user_asm_byte(__gu_val, __gu_addr, err, __t); break;	\
-	case 2:	__get_user_asm_half(__gu_val, __gu_addr, err, __t); break;	\
-	case 4:	__get_user_asm_word(__gu_val, __gu_addr, err, __t); break;	\
-	default: (__gu_val) = __get_user_bad();				\
+	case 1:	{							\
+		u8 __gu_val;						\
+		__get_user_asm_byte(__gu_val, __gu_addr, err, __t);	\
+		(x) = *(__force __typeof__(*(ptr)) *) &__gu_val;	\
+		break;							\
+	}								\
+	case 2:	{							\
+		u16 __gu_val;						\
+		__get_user_asm_half(__gu_val, __gu_addr, err, __t);	\
+		(x) = *(__force __typeof__(*(ptr)) *) &__gu_val;	\
+		break;							\
+	}								\
+	case 4:	{							\
+		u32 __gu_val;						\
+		__get_user_asm_word(__gu_val, __gu_addr, err, __t);	\
+		(x) = *(__force __typeof__(*(ptr)) *) &__gu_val;	\
+		break;							\
+	}								\
+	case 8:	{							\
+		u64 __gu_val;						\
+		__get_user_asm_dword(__gu_val, __gu_addr, err, __t);	\
+		(x) = *(__force __typeof__(*(ptr)) *) &__gu_val;	\
+		break;							\
+	}								\
+	default: {							\
+		unsigned long __gu_val;					\
+		(__gu_val) = __get_user_bad();				\
+	}								\
 	}								\
 	uaccess_restore(__ua_flags);					\
-	(x) = (__typeof__(*(ptr)))__gu_val;				\
 } while (0)
 #endif
 
@@ -353,6 +375,22 @@ do {									\
 #define __get_user_asm_word(x, addr, err, __t)			\
 	__get_user_asm(x, addr, err, "ldr" __t)
 
+#ifdef __ARMEB__
+#define __WORD0_OFFS	4
+#define __WORD1_OFFS	0
+#else
+#define __WORD0_OFFS	0
+#define __WORD1_OFFS	4
+#endif
+
+#define __get_user_asm_dword(x, addr, err, __t)				\
+	({								\
+	unsigned long __w0, __w1;					\
+	__get_user_asm(__w0, addr + __WORD0_OFFS, err, "ldr" __t);	\
+	__get_user_asm(__w1, addr + __WORD1_OFFS, err, "ldr" __t);	\
+	(x) = ((u64)__w1 << 32) | (u64) __w0;				\
+})
+
 #define __put_user_switch(x, ptr, __err, __fn)				\
 	do {								\
 		const __typeof__(*(ptr)) __user *__pu_ptr = (ptr);	\

Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Russell King (Oracle)]

On Wed, Sep 17, 2025 at 03:55:10PM +0200, Thomas Gleixner wrote:
> On Wed, Sep 17 2025 at 10:41, Russell King wrote:
> > On Wed, Sep 17, 2025 at 07:48:00AM +0200, Thomas Gleixner wrote:
> >
> > Putting together a simple test case, where the only change is making
> > __gu_val an unsigned long long:
> >
> > t.c: In function ‘get_ptr’:
> > t.c:40:15: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> >    40 |         (x) = (__typeof__(*(ptr)))__gu_val;                             \
> >       |               ^
> > t.c:21:9: note: in expansion of macro ‘__get_user_err’
> >    21 |         __get_user_err((x), (ptr), __gu_err, TUSER());                  \
> >       |         ^~~~~~~~~~~~~~
> > t.c:102:16: note: in expansion of macro ‘__get_user’
> >   102 |         return __get_user(p, ptr);
> >       |                ^~~~~~~~~~
> >
> > In order for the code you are modifying to be reachable, you need to
> > build with CONFIG_CPU_SPECTRE disabled. This is produced by:
> >
> > int get_ptr(void **ptr)
> > {
> >         void *p;
> >
> >         return __get_user(p, ptr);
> > }
> 
> Duh, yes. I hate get_user() and I did not notice, because the
> allmodconfig build breaks early due to frame size checks, so I was too
> lazy to find that config knob and built only a couple of things and an
> artificial test case for u64.
> 
> But it actually can be solved solvable by switching the casting to:
> 
>     (x) = *(__force __typeof__(*(ptr)) *) &__gu_val;
> 
> Not pretty, but after upping the frame size limit it builds an
> allmodconfig kernel.

For me, this produces:

get-user-test.c:41:16: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
   41 |         (x) = *(__force __typeof__(*(ptr)) *) &__gu_val;                \
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

with arm-linux-gnueabihf-gcc (Debian 14.2.0-19) 14.2.0

Maybe you're using a different compiler that doesn't issue that warning?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Nathan Chancellor]

On Wed, Sep 17, 2025 at 04:17:38PM +0100, Russell King (Oracle) wrote:
> For me, this produces:
> 
> get-user-test.c:41:16: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
>    41 |         (x) = *(__force __typeof__(*(ptr)) *) &__gu_val;                \
>       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> with arm-linux-gnueabihf-gcc (Debian 14.2.0-19) 14.2.0
> 
> Maybe you're using a different compiler that doesn't issue that warning?

Maybe because the kernel uses -fno-strict-aliasing, which presumably
turns off -Wstrict-aliasing?

Cheers,
Nathan

Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Russell King (Oracle)]

On Wed, Sep 17, 2025 at 10:14:24AM -0700, Nathan Chancellor wrote:
> On Wed, Sep 17, 2025 at 04:17:38PM +0100, Russell King (Oracle) wrote:
> > For me, this produces:
> > 
> > get-user-test.c:41:16: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
> >    41 |         (x) = *(__force __typeof__(*(ptr)) *) &__gu_val;                \
> >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > with arm-linux-gnueabihf-gcc (Debian 14.2.0-19) 14.2.0
> > 
> > Maybe you're using a different compiler that doesn't issue that warning?
> 
> Maybe because the kernel uses -fno-strict-aliasing, which presumably
> turns off -Wstrict-aliasing?

Thanks, I'd forgotten to pick the -f flags for building the out of tree
test. Yes, that does work, but I wonder whether the powerpc 32-bit
approach with __long_type() that Christophe mentioned would be better.
That also appears to avoid all issues, and doesn't need the use of
the nasty __force, address-of and deref trick.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Thomas Gleixner]

On Wed, Sep 17 2025 at 16:17, Russell King wrote:
> On Wed, Sep 17, 2025 at 03:55:10PM +0200, Thomas Gleixner wrote:
>> But it actually can be solved solvable by switching the casting to:
>> 
>>     (x) = *(__force __typeof__(*(ptr)) *) &__gu_val;
>> 
>> Not pretty, but after upping the frame size limit it builds an
>> allmodconfig kernel.
>
> For me, this produces:
>
> get-user-test.c:41:16: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
>    41 |         (x) = *(__force __typeof__(*(ptr)) *) &__gu_val;                \
>       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> with arm-linux-gnueabihf-gcc (Debian 14.2.0-19) 14.2.0
>
> Maybe you're using a different compiler that doesn't issue that
> warning?

Yes :)

Was this with the one-line change or with the full conversion?

Thanks,

        tglx

Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Thomas Gleixner]

On Wed, Sep 17 2025 at 18:34, Russell King wrote:

> On Wed, Sep 17, 2025 at 10:14:24AM -0700, Nathan Chancellor wrote:
>> On Wed, Sep 17, 2025 at 04:17:38PM +0100, Russell King (Oracle) wrote:
>> > For me, this produces:
>> > 
>> > get-user-test.c:41:16: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
>> >    41 |         (x) = *(__force __typeof__(*(ptr)) *) &__gu_val;                \
>> >       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > 
>> > with arm-linux-gnueabihf-gcc (Debian 14.2.0-19) 14.2.0
>> > 
>> > Maybe you're using a different compiler that doesn't issue that warning?
>> 
>> Maybe because the kernel uses -fno-strict-aliasing, which presumably
>> turns off -Wstrict-aliasing?
>
> Thanks, I'd forgotten to pick the -f flags for building the out of tree
> test. Yes, that does work, but I wonder whether the powerpc 32-bit
> approach with __long_type() that Christophe mentioned would be better.
> That also appears to avoid all issues, and doesn't need the use of
> the nasty __force, address-of and deref trick.

Hmm. I just noticed that include/asm-generic/uaccess.h does exactly the
same what I did with the per size case variables and that seems to be
not subject to endless bot complaints either.

But sure, the PPC trick is neat too. No strong preference, just that I'm
leaning towards the per case split up as it's less obfuscated IMO.

Thanks,

        tglx

Re: [patch V2 3/6] uaccess: Provide scoped masked user access regions

[Mathieu Desnoyers]

On 16-Sep-2025 06:33:13 PM, Thomas Gleixner wrote:
> User space access regions are tedious and require similar code patterns all
> over the place:
> 
>      	if (!user_read_access_begin(from, sizeof(*from)))
> 		return -EFAULT;
> 	unsafe_get_user(val, from, Efault);
> 	user_read_access_end();
> 	return 0;
> Efault:
> 	user_read_access_end();
> 	return -EFAULT;
> 
> This got worse with the recend addition of masked user access, which
> optimizes the speculation prevention:
> 
> 	if (can_do_masked_user_access())
> 		from = masked_user_read_access_begin((from));
> 	else if (!user_read_access_begin(from, sizeof(*from)))
> 		return -EFAULT;
> 	unsafe_get_user(val, from, Efault);
> 	user_read_access_end();
> 	return 0;
> Efault:
> 	user_read_access_end();
> 	return -EFAULT;
> 
> There have been issues with using the wrong user_*_access_end() variant in
> the error path and other typical Copy&Pasta problems, e.g. using the wrong
> fault label in the user accessor which ends up using the wrong accesss end
> variant. 
> 
> These patterns beg for scopes with automatic cleanup. The resulting outcome
> is:
>     	scoped_masked_user_read_access(from, return -EFAULT,
> 		scoped_get_user(val, from); );
> 	return 0;

I find a few aspects of the proposed API odd:

- Explicitly implementing the error label within a macro parameter,
- Having the scoped code within another macro parameter.

I would rather expect something like this to mimick our expectations
in C:

int func(void __user *ptr, size_t len, char *val1, char *val2)
{
        int ret;

        scoped_masked_user_read_access(ptr, len, ret) {
                scoped_get_user(val1, ptr[0]);
                scoped_get_user(val2, ptr[0]);
        }
        return ret;
}

Where:

- ptr is the pointer at the beginning of the range where the userspace
  access will be done.
- len is the length of the range.
- ret is a variable used as output (set to -EFAULT on error, 0 on
  success). If the user needs to do something cleverer than
  get a -EFAULT on error, they can open-code it rather than use
  the scoped helper.
- The scope is presented similarly to a "for ()" loop scope.

Now I have no clue whether preprocessor limitations prevent achieving
this somehow, or if it would end up generating poor assembler.

Thoughts ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [patch V2 3/6] uaccess: Provide scoped masked user access regions

[Thomas Gleixner]

On Thu, Sep 18 2025 at 09:20, Mathieu Desnoyers wrote:
> On 16-Sep-2025 06:33:13 PM, Thomas Gleixner wrote:
>> These patterns beg for scopes with automatic cleanup. The resulting outcome
>> is:
>>     	scoped_masked_user_read_access(from, return -EFAULT,
>> 		scoped_get_user(val, from); );
>> 	return 0;
>
> I find a few aspects of the proposed API odd:
>
> - Explicitly implementing the error label within a macro parameter,

Which error label are you talking about?

> - Having the scoped code within another macro parameter.

Macros are limited and the whole construct requires a closed scope to
work and to keep the local variables and the jump label local.

> I would rather expect something like this to mimick our expectations
> in C:

I obviously would love to do it more C style as everyone else.
If you can come up with a way to do that in full C I'm all ears :)

> int func(void __user *ptr, size_t len, char *val1, char *val2)
> {
>         int ret;
>
>         scoped_masked_user_read_access(ptr, len, ret) {
>                 scoped_get_user(val1, ptr[0]);
>                 scoped_get_user(val2, ptr[0]);
>         }
>         return ret;
> }
>
> Where:
>
> - ptr is the pointer at the beginning of the range where the userspace
>   access will be done.

That's the case with the proposed interface already, no?

> - len is the length of the range.

The majority of use cases does not need an explicit size because the
size is determined by the data type. So not forcing everyone to write
scope(ptr, sizeof(*ptr), ..) is a good thing, no?

Adding a sized interface on top for the others is straight forward
enough.

> - ret is a variable used as output (set to -EFAULT on error, 0 on
>   success). If the user needs to do something cleverer than
>   get a -EFAULT on error, they can open-code it rather than use
>   the scoped helper.

Just write  "ret = WHATEVER" instead of "return WHATEVER".

> - The scope is presented similarly to a "for ()" loop scope.

It is a loop scope and you can exit it with either return or break.

> Now I have no clue whether preprocessor limitations prevent achieving
> this somehow, or if it would end up generating poor assembler.

There is no real good way to implement it so that the scope local
requirements are fulfilled. The only alternative would be to close the
scope with a bracket after the code:

      scope(....)
        foo(); }

or for multiline:

      scope(....) {
           ....
      ))

The lonely extra bracket screws up reading even more than the code
within the macro parameter because it's imbalanced. It also makes
tooling from editors to code checkers mightily unhappy.

Thanks,

        tglx

[patch V2a 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()

[Thomas Gleixner]

When CONFIG_CPU_SPECTRE=n then get_user() is missing the 8 byte ASM
variant.  This prevents using get_user(u64) in generic code.

Implement it as a sequence of two 4-byte reads with LE/BE awareness and
make the data type for the intermediate variable to read into dependend
on the target type. For 8,16,32 bit use unsigned long and for 64 bit
unsigned long long

The __long_type() macro and the idea to solve it was lifted from
PowerPC. Thanks to Christophe for pointing it out.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Closes: https://lore.kernel.org/oe-kbuild-all/202509120155.pFgwfeUD-lkp@intel.com/
---
V2a: Solve the *ptr issue vs. unsigned long long - Russell/Christophe
V2: New patch to fix the 0-day fallout
---
 arch/arm/include/asm/uaccess.h |   26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -283,10 +283,17 @@ extern int __put_user_8(void *, unsigned
 	__gu_err;							\
 })
 
+/*
+ * This is a type: either unsigned long, if the argument fits into
+ * that type, or otherwise unsigned long long.
+ */
+#define __long_type(x) \
+	__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
+
 #define __get_user_err(x, ptr, err, __t)				\
 do {									\
 	unsigned long __gu_addr = (unsigned long)(ptr);			\
-	unsigned long __gu_val;						\
+	__long_type(x) __gu_val;					\
 	unsigned int __ua_flags;					\
 	__chk_user_ptr(ptr);						\
 	might_fault();							\
@@ -295,6 +302,7 @@ do {									\
 	case 1:	__get_user_asm_byte(__gu_val, __gu_addr, err, __t); break;	\
 	case 2:	__get_user_asm_half(__gu_val, __gu_addr, err, __t); break;	\
 	case 4:	__get_user_asm_word(__gu_val, __gu_addr, err, __t); break;	\
+	case 8:	__get_user_asm_dword(__gu_val, __gu_addr, err, __t); break;	\
 	default: (__gu_val) = __get_user_bad();				\
 	}								\
 	uaccess_restore(__ua_flags);					\
@@ -353,6 +361,22 @@ do {									\
 #define __get_user_asm_word(x, addr, err, __t)			\
 	__get_user_asm(x, addr, err, "ldr" __t)
 
+#ifdef __ARMEB__
+#define __WORD0_OFFS	4
+#define __WORD1_OFFS	0
+#else
+#define __WORD0_OFFS	0
+#define __WORD1_OFFS	4
+#endif
+
+#define __get_user_asm_dword(x, addr, err, __t)				\
+	({								\
+	unsigned long __w0, __w1;					\
+	__get_user_asm(__w0, addr + __WORD0_OFFS, err, "ldr" __t);	\
+	__get_user_asm(__w1, addr + __WORD1_OFFS, err, "ldr" __t);	\
+	(x) = ((u64)__w1 << 32) | (u64) __w0;				\
+})
+
 #define __put_user_switch(x, ptr, __err, __fn)				\
 	do {								\
 		const __typeof__(*(ptr)) __user *__pu_ptr = (ptr);	\

Re: [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17

[Geert Uytterhoeven]

Hi Thomas,

On Wed, 17 Sept 2025 at 07:51, Thomas Gleixner <tglx@linutronix.de> wrote:
> clang < 17 fails to use scope local labels with CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y:
>
>      {
>         __label__ local_lbl;
>         ...
>         unsafe_get_user(uval, uaddr, local_lbl);
>         ...
>         return 0;
>         local_lbl:
>                 return -EFAULT;
>      }
>
> when two such scopes exist in the same function:
>
>   error: cannot jump from this asm goto statement to one of its possible targets
>
> There are other failure scenarios. Shuffling code around slightly makes it
> worse and fail even with one instance.
>
> That issue prevents using local labels for a cleanup based user access
> mechanism.
>
> After failed attempts to provide a simple enough test case for the 'depends
> on' test in Kconfig, the initial cure was to mark ASM goto broken on clang
> versions < 17 to get this road block out of the way.
>
> But Nathan pointed out that this is a known clang issue and indeed affects
> clang < version 17 in combination with cleanup(). It's not even required to
> use local labels for that.
>
> The clang issue tracker has a small enough test case, which can be used as
> a test in the 'depends on' section of CC_HAS_ASM_GOTO_OUTPUT:
>
> void bar(void **);
> void* baz();
>
> int  foo (void) {
>     {
>             asm goto("jmp %l0"::::l0);
>             return 0;
> l0:
>             return 1;
>     }
>     void *x __attribute__((cleanup(bar))) = baz();
>     {
>             asm goto("jmp %l0"::::l1);
>             return 42;
> l1:
>             return 0xff;
>     }
> }
>
> Add another dependency to config CC_HAS_ASM_GOTO_OUTPUT for it and use the
> clang issue tracker test case for detection by condensing it to obfuscated
> C-code contest format. This reliably catches the problem on clang < 17 and
> did not show any issues on the non known to be broken GCC versions.
>
> That test might be sufficient to catch all issues and therefore could
> replace the existing test, but keeping that around does no harm either.
>
> Thanks to Nathan for pointing to the relevant clang issue!
>
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1886
> Link: https://github.com/llvm/llvm-project/commit/f023f5cdb2e6c19026f04a15b5a935c041835d14

Thanks for your patch, which is now commit e2ffa15b9baa447e ("kbuild:
Disable CC_HAS_ASM_GOTO_OUTPUT on clang < 17") in v6.17.

> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -99,7 +99,10 @@ config GCC_ASM_GOTO_OUTPUT_BROKEN
>  config CC_HAS_ASM_GOTO_OUTPUT
>         def_bool y
>         depends on !GCC_ASM_GOTO_OUTPUT_BROKEN
> +       # Find basic issues
>         depends on $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)
> +       # Detect buggy clang, fixed in clang-17
> +       depends on $(success,echo 'void b(void **);void* c();int f(void){{asm goto("jmp %l0"::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto("jmp %l0"::::l1);return 2;l1:return 1;}}' | $(CC) -x c - -c -o /dev/null)

This is supposed to affect only clang builds, right?  I am using
gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04) to build for
arm32/arm64/riscv, and thus have:

    CONFIG_CC_IS_GCC=y

Still, this commit causes

    CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y
    CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT=y

to disappear from my configs? Is that expected?

Thanks!

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17

[Peter Zijlstra]

On Mon, Sep 29, 2025 at 11:38:17AM +0200, Geert Uytterhoeven wrote:

> > +       # Detect buggy clang, fixed in clang-17
> > +       depends on $(success,echo 'void b(void **);void* c();int f(void){{asm goto("jmp %l0"::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto("jmp %l0"::::l1);return 2;l1:return 1;}}' | $(CC) -x c - -c -o /dev/null)
> 
> This is supposed to affect only clang builds, right?  I am using
> gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04) to build for
> arm32/arm64/riscv, and thus have:
> 
>     CONFIG_CC_IS_GCC=y
> 
> Still, this commit causes
> 
>     CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y
>     CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT=y
> 
> to disappear from my configs? Is that expected?

Not expected -- that means your GCC is somehow failing that test case.
Ideally some GCC person will investigate why this is so.

Re: [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17

[Geert Uytterhoeven]

On Mon, 29 Sept 2025 at 12:09, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Sep 29, 2025 at 11:38:17AM +0200, Geert Uytterhoeven wrote:
>
> > > +       # Detect buggy clang, fixed in clang-17
> > > +       depends on $(success,echo 'void b(void **);void* c();int f(void){{asm goto("jmp %l0"::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto("jmp %l0"::::l1);return 2;l1:return 1;}}' | $(CC) -x c - -c -o /dev/null)
> >
> > This is supposed to affect only clang builds, right?  I am using
> > gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04) to build for
> > arm32/arm64/riscv, and thus have:
> >
> >     CONFIG_CC_IS_GCC=y
> >
> > Still, this commit causes
> >
> >     CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y
> >     CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT=y
> >
> > to disappear from my configs? Is that expected?
>
> Not expected -- that means your GCC is somehow failing that test case.
> Ideally some GCC person will investigate why this is so.

Oh, "jmp" is not a valid mnemonic on arm and riscv, and several other
architectures...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17

[Peter Zijlstra]

On Mon, Sep 29, 2025 at 12:58:14PM +0200, Geert Uytterhoeven wrote:
> On Mon, 29 Sept 2025 at 12:09, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Mon, Sep 29, 2025 at 11:38:17AM +0200, Geert Uytterhoeven wrote:
> >
> > > > +       # Detect buggy clang, fixed in clang-17
> > > > +       depends on $(success,echo 'void b(void **);void* c();int f(void){{asm goto("jmp %l0"::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto("jmp %l0"::::l1);return 2;l1:return 1;}}' | $(CC) -x c - -c -o /dev/null)
> > >
> > > This is supposed to affect only clang builds, right?  I am using
> > > gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04) to build for
> > > arm32/arm64/riscv, and thus have:
> > >
> > >     CONFIG_CC_IS_GCC=y
> > >
> > > Still, this commit causes
> > >
> > >     CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y
> > >     CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT=y
> > >
> > > to disappear from my configs? Is that expected?
> >
> > Not expected -- that means your GCC is somehow failing that test case.
> > Ideally some GCC person will investigate why this is so.
> 
> Oh, "jmp" is not a valid mnemonic on arm and riscv, and several other
> architectures...

Ah, d'0h indeed.

void b(void **);void* c();int f(void){{asm goto(""::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto(""::::l1);return 2;l1:return 1;}}

Seems to still finger the issue on x86_64. That should build on !x86
too, right?

Re: [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17

[Geert Uytterhoeven]

Hoi Peter,

On Mon, 29 Sept 2025 at 13:04, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Sep 29, 2025 at 12:58:14PM +0200, Geert Uytterhoeven wrote:
> > On Mon, 29 Sept 2025 at 12:09, Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Mon, Sep 29, 2025 at 11:38:17AM +0200, Geert Uytterhoeven wrote:
> > >
> > > > > +       # Detect buggy clang, fixed in clang-17
> > > > > +       depends on $(success,echo 'void b(void **);void* c();int f(void){{asm goto("jmp %l0"::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto("jmp %l0"::::l1);return 2;l1:return 1;}}' | $(CC) -x c - -c -o /dev/null)
> > > >
> > > > This is supposed to affect only clang builds, right?  I am using
> > > > gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04) to build for
> > > > arm32/arm64/riscv, and thus have:
> > > >
> > > >     CONFIG_CC_IS_GCC=y
> > > >
> > > > Still, this commit causes
> > > >
> > > >     CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y
> > > >     CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT=y
> > > >
> > > > to disappear from my configs? Is that expected?
> > >
> > > Not expected -- that means your GCC is somehow failing that test case.
> > > Ideally some GCC person will investigate why this is so.
> >
> > Oh, "jmp" is not a valid mnemonic on arm and riscv, and several other
> > architectures...
>
> Ah, d'0h indeed.
>
> void b(void **);void* c();int f(void){{asm goto(""::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto(""::::l1);return 2;l1:return 1;}}
>
> Seems to still finger the issue on x86_64. That should build on !x86
> too, right?

Thanks, builds fine on arm32, arm64, riscv, m68k, powerpc, mips, s390.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17

[Linus Torvalds]

On Mon, 29 Sept 2025 at 04:10, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> >
> > Ah, d'0h indeed.
> >
> > void b(void **);void* c();int f(void){{asm goto(""::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto(""::::l1);return 2;l1:return 1;}}
> >
> > Seems to still finger the issue on x86_64. That should build on !x86
> > too, right?
>
> Thanks, builds fine on arm32, arm64, riscv, m68k, powerpc, mips, s390.

Ok, I just applied that fix directly. It's clearly not a fatal bug
since it just falls back on the non-optimal code, but it's one of
those "silly and subtle code generation issues" so I'd rather have it
fixed asap in the upstream kernel.

Geert, thanks for noticing.

             Linus

Re: [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17

[Thomas Gleixner]

On Mon, Sep 29 2025 at 13:04, Peter Zijlstra wrote:
> On Mon, Sep 29, 2025 at 12:58:14PM +0200, Geert Uytterhoeven wrote:
>> On Mon, 29 Sept 2025 at 12:09, Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Mon, Sep 29, 2025 at 11:38:17AM +0200, Geert Uytterhoeven wrote:
>> >
>> > > > +       # Detect buggy clang, fixed in clang-17
>> > > > +       depends on $(success,echo 'void b(void **);void* c();int f(void){{asm goto("jmp %l0"::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto("jmp %l0"::::l1);return 2;l1:return 1;}}' | $(CC) -x c - -c -o /dev/null)
>> > >
>> > > This is supposed to affect only clang builds, right?  I am using
>> > > gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04) to build for
>> > > arm32/arm64/riscv, and thus have:
>> > >
>> > >     CONFIG_CC_IS_GCC=y
>> > >
>> > > Still, this commit causes
>> > >
>> > >     CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y
>> > >     CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT=y
>> > >
>> > > to disappear from my configs? Is that expected?
>> >
>> > Not expected -- that means your GCC is somehow failing that test case.
>> > Ideally some GCC person will investigate why this is so.
>> 
>> Oh, "jmp" is not a valid mnemonic on arm and riscv, and several other
>> architectures...
>
> Ah, d'0h indeed.
>
> void b(void **);void* c();int f(void){{asm goto(""::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto(""::::l1);return 2;l1:return 1;}}
>
> Seems to still finger the issue on x86_64. That should build on !x86
> too, right?

Ooops.

Thanks for the quick fix!

       tglx

Re: [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17

[David Laight]

On Mon, 29 Sep 2025 08:53:37 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Mon, 29 Sept 2025 at 04:10, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >  
> > >
> > > Ah, d'0h indeed.
> > >
> > > void b(void **);void* c();int f(void){{asm goto(""::::l0);return 0;l0:return 1;}void *x __attribute__((cleanup(b))) = c();{asm goto(""::::l1);return 2;l1:return 1;}}

Should that be 'void *c(void);' (with an extra void) to avoid failing because
of the K&R function declaration?

	David

> > >
> > > Seems to still finger the issue on x86_64. That should build on !x86
> > > too, right?  
> >
> > Thanks, builds fine on arm32, arm64, riscv, m68k, powerpc, mips, s390.  
> 
> Ok, I just applied that fix directly. It's clearly not a fatal bug
> since it just falls back on the non-optimal code, but it's one of
> those "silly and subtle code generation issues" so I'd rather have it
> fixed asap in the upstream kernel.
> 
> Geert, thanks for noticing.
> 
>              Linus
>