# Re: \[RFC PATCH v2 18/24\] x86/uaccess: Add unsafe\_copy\_from\_user() implementation

- From: Jens Remus \<hidden\>
- Date: 2026-09-14 14:54:03 
- List: linux-trace-kernel
- Page: https://ratatoskr.run/linux-trace-kernel/2026/09/17574123
- Thread: https://ratatoskr.run/sashiko-reviews/2026/09/17574123/t.md

```text
On 8/21/2026 10:11 PM, sashiko-bot@kernel.org wrote:

> This commit adds an x86 implementation of unsafe_copy_from_user() similar to
> the existing unsafe_copy_to_user() macro. It also updates the existing
> unsafe_copy_to_user() macro to use shorter internal variable names.

>> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h

>> -#define unsafe_copy_to_user(_dst,_src,_len,label)			\
>> -do {									\
>> -	char __user *__ucu_dst = (_dst);				\
>> -	const char *__ucu_src = (_src);					\
>> -	size_t __ucu_len = (_len);					\
>> -	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u64, label);	\
>> -	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u32, label);	\
>> -	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u16, label);	\
>> -	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u8, label);	\
>> +#define unsafe_copy_to_user(_dst, _src, _len, label)				\
>> +do {										\
>> +	void __user *__dst = (_dst);						\
>> +	const void *__src = (_src);						\
>> +	size_t __len = (_len);							\
> 
> [Severity: Medium]
> Could this macro change cause dangerous variable shadowing?
> 
> By renaming the internal variables from __ucu_dst to generic names like __dst,
> __src, and __len, if a caller passes arguments with those exact names, the
> macro will expand to something like void __user *__dst = (__dst);. This
> results in self-initialization using uninitialized stack memory, which could
> lead to arbitrary memory reads or writes when used with SMAP disabled.

IIRC Steven advised to disregard Sashiko's concerns and leave Josh's
commit as-is for now.  Any objections?

> 
>> +	unsafe_copy_to_user_loop(__dst, __src, __len, u64, label);		\
>> +	unsafe_copy_to_user_loop(__dst, __src, __len, u32, label);		\
>> +	unsafe_copy_to_user_loop(__dst, __src, __len, u16, label);		\
>> +	unsafe_copy_to_user_loop(__dst, __src, __len, u8,  label);		\
>> +} while (0)
>> +
>> +#define unsafe_copy_from_user_loop(dst, src, len, type, label)			\
>> +	while (len >= sizeof(type)) {						\
>> +		unsafe_get_user(*(type *)(dst), (type __user *)(src), label);	\
>> +		dst += sizeof(type);						\
>> +		src += sizeof(type);						\
>> +		len -= sizeof(type);						\
>> +	}
>> +
>> +#define unsafe_copy_from_user(_dst, _src, _len, label)				\
>> +do {										\
>> +	void *__dst = (_dst);							\
>> +	const void __user *__src = (_src);					\
>> +	size_t __len = (_len);							\
> 
> [Severity: Medium]
> Does this new macro suffer from the same variable shadowing risk?
> 
> Using generic internal names like __dst, __src, and __len might cause
> accidental self-initialization if a caller happens to use the same variable
> names for the arguments. Should we use more uniquely prefixed names here
> similar to the old __ucu_dst?

Likewise.

> 
>> +	unsafe_copy_from_user_loop(__dst, __src, __len, u64, label);		\
>> +	unsafe_copy_from_user_loop(__dst, __src, __len, u32, label);		\
>> +	unsafe_copy_from_user_loop(__dst, __src, __len, u16, label);		\
>> +	unsafe_copy_from_user_loop(__dst, __src, __len, u8,  label);		\
>>  } while (0)
Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

```