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.
quoted
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
quoted
-#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?
quoted
+ 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.
quoted
+ 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/