Re: [RFC PATCH v1 19/25] x86/uaccess: Add unsafe_copy_from_user() implementation
From: sashiko-bot@kernel.org
Date: 2026-08-18 15:08:07
Also in:
linux-s390
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The `unsafe_copy_to_user` and `unsafe_copy_from_user` macros use dangerously common local variable names (`__dst`, `__src`, `__len`) which can silently shadow caller variables. --
--- Patch [19]: [RFC PATCH v1 19/25] x86/uaccess: Add unsafe_copy_from_user() implementation ---commit 83e5aa488f68698873050ec00671487d755924f5 Author: Josh Poimboeuf [off-list ref] x86/uaccess: Add unsafe_copy_from_user() implementation This commit adds an x86 implementation of unsafe_copy_from_user() similar to the existing unsafe_copy_to_user(). It also updates the local variables in the unsafe_copy_to_user() macro.
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h --- 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 these generic variable names (__dst, __src, __len) silently shadow variables in the caller's scope? If a caller passes a variable named __len as the _len argument, the macro will expand to size_t __len = (__len);. This declares a new block-scoped variable and initializes it with its own uninitialized garbage value from the stack. The copy loop would then use this uninitialized length, potentially resulting in an out-of-bounds memory copy. Does it make sense to retain a unique prefix like the previous __ucu_ variable names to prevent macro shadowing?
+ 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(_dst, _src, _len, label) \
+do { \
+ void *__dst = (_dst); \
+ const void __user *__src = (_src); \
+ size_t __len = (_len); \[Severity: Medium] Similarly, does the new unsafe_copy_from_user() macro share the same shadowing vulnerability?
+ 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)
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260818144954.2320378-1-jremus@linux.ibm.com?part=19