On 8/21/2026 10:11 PM, [email protected] 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)
[email protected] / [email protected]
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/
