linux-kernel - Re: [patch V3 01/12] ARM: uaccess: Implement missing __get_user_asm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8669c9f6-74c3-4bd6-833c-1d73158dfc97@efficios.com>
Date: Fri, 17 Oct 2025 08:36:50 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Thomas Gleixner <tglx@...utronix.de>, LKML <linux-kernel@...r.kernel.org>
Cc: kernel test robot <lkp@...el.com>, Russell King <linux@...linux.org.uk>,
 linux-arm-kernel@...ts.infradead.org,
 Linus Torvalds <torvalds@...ux-foundation.org>, x86@...nel.org,
 Madhavan Srinivasan <maddy@...ux.ibm.com>,
 Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 linuxppc-dev@...ts.ozlabs.org, Paul Walmsley <pjw@...nel.org>,
 Palmer Dabbelt <palmer@...belt.com>, linux-riscv@...ts.infradead.org,
 Heiko Carstens <hca@...ux.ibm.com>,
 Christian Borntraeger <borntraeger@...ux.ibm.com>,
 Sven Schnelle <svens@...ux.ibm.com>, linux-s390@...r.kernel.org,
 Andrew Cooper <andrew.cooper3@...rix.com>,
 Julia Lawall <Julia.Lawall@...ia.fr>, Nicolas Palix <nicolas.palix@...g.fr>,
 Peter Zijlstra <peterz@...radead.org>, Darren Hart <dvhart@...radead.org>,
 Davidlohr Bueso <dave@...olabs.net>, André Almeida
 <andrealmeid@...lia.com>, Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 linux-fsdevel@...r.kernel.org
Subject: Re: [patch V3 01/12] ARM: uaccess: Implement missing
 __get_user_asm_dword()

On 2025-10-17 06:08, 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.
> 
> Implement it as a sequence of two 4-byte reads with LE/BE awareness and
> make the unsigned long (or long long) type for the intermediate variable to
> read into dependend on the the target type.
> 
> The __long_type() macro and idea was lifted from PowerPC. Thanks to
> Christophe for pointing it out.
> 
> Reported-by: kernel test robot <lkp@...el.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Cc: Russell King <linux@...linux.org.uk>
> Cc: linux-arm-kernel@...ts.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;				\
> +})

If we look at __get_user_asm_half, it always loads the lower addresses
first (__gu_addr), and then loads the following address (__gu_addr + 1).

This new code for dword flips the order of word accesses between BE and
LE, which means that on BE we're reading the second word and then moving
back one word.

I'm not sure whether it matters or not, but I'm pointing it out in case
it matters in terms of hardware memory access pattern.

Also we end up with __get_user_asm_{half,dword} that effectively do the
same tricks in very different ways, so it would be good to come up with
a unified pattern.

Thanks,

Mathieu


> +
>   #define __put_user_switch(x, ptr, __err, __fn)				\
>   	do {								\
>   		const __typeof__(*(ptr)) __user *__pu_ptr = (ptr);	\
> 


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