linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"Linus Torvalds" <torvalds@linux-foundation.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"kernel test robot" <lkp@intel.com>,
	linux-arm-kernel@lists.infradead.org,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Christophe Leroy" <christophe.leroy@csgroup.eu>,
	"Darren Hart" <dvhart@infradead.org>,
	"Davidlohr Bueso" <dave@stgolabs.net>,
	"André Almeida" <andrealmeid@igalia.com>,
	x86@kernel.org, "Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	"Jan Kara" <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword()
Date: Wed, 17 Sep 2025 15:55:10 +0200	[thread overview]
Message-ID: <87y0qd89q9.ffs@tglx> (raw)
In-Reply-To: <aMqCPVmOArg8dIqR@shell.armlinux.org.uk>

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);	\




  parent reply	other threads:[~2025-09-17 13:55 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-16 16:33 [patch V2 0/6] uaccess: Provide and use scopes for user masked access Thomas Gleixner
2025-09-16 16:33 ` [patch V2 1/6] ARM: uaccess: Implement missing __get_user_asm_dword() Thomas Gleixner
2025-09-16 21:26   ` Russell King (Oracle)
2025-09-17  5:48     ` Thomas Gleixner
2025-09-17  9:41       ` Russell King (Oracle)
2025-09-17 12:35         ` Christophe Leroy
2025-09-17 13:55         ` Thomas Gleixner [this message]
2025-09-17 15:17           ` Russell King (Oracle)
2025-09-17 17:14             ` Nathan Chancellor
2025-09-17 17:34               ` Russell King (Oracle)
2025-09-17 19:25                 ` Thomas Gleixner
2025-09-17 18:44             ` Thomas Gleixner
2025-09-19 18:27   ` [patch V2a " Thomas Gleixner
2025-09-16 16:33 ` [patch V2 2/6] kbuild: Disable asm goto on clang < 17 Thomas Gleixner
2025-09-16 18:44   ` Nathan Chancellor
2025-09-16 20:43     ` Thomas Gleixner
2025-09-16 20:56       ` [patch V2a 2/6] kbuild: Disable CC_HAS_ASM_GOTO_OUTPUT on clang < version 17 Thomas Gleixner
2025-09-16 21:50         ` Nathan Chancellor
2025-09-29  9:38         ` Geert Uytterhoeven
2025-09-29 10:08           ` Peter Zijlstra
2025-09-29 10:58             ` Geert Uytterhoeven
2025-09-29 11:04               ` Peter Zijlstra
2025-09-29 11:10                 ` Geert Uytterhoeven
2025-09-29 15:53                   ` Linus Torvalds
2025-10-02 18:47                     ` David Laight
2025-09-29 22:05                 ` Thomas Gleixner
2025-09-16 16:33 ` [patch V2 3/6] uaccess: Provide scoped masked user access regions Thomas Gleixner
2025-09-18 13:20   ` Mathieu Desnoyers
2025-09-19  9:10     ` Thomas Gleixner
2025-09-16 16:33 ` [patch V2 4/6] futex: Convert to scoped masked user access Thomas Gleixner
2025-09-16 16:33 ` [patch V2 5/6] x86/futex: " Thomas Gleixner
2025-09-16 16:33 ` [patch V2 6/6] select: " Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y0qd89q9.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=andrealmeid@igalia.com \
    --cc=brauner@kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=dave@stgolabs.net \
    --cc=dvhart@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=lkp@intel.com \
    --cc=nathan@kernel.org \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).