Re: [PATCH v14 04/19] x86/uaccess: Add unsafe_copy_from_user() implementation

On 5/5/2026 8:22 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Variable shadowing hazard in unsafe_copy_to_user and unsafe_copy_from_user macros.
> - [Low] Missing const qualifier on the source pointer in unsafe_copy_from_user.
> --
> 
> Note: The format of this report is altered due to recitation restrictions.
> The original patch code is not quoted directly, and a free-form summary
> of the review findings is provided instead.
> 
> commit b652061f302d029392e67f46699e5a2db0abfaca
> Author: Josh Poimboeuf <[email protected]>
> 
> x86/uaccess: Add unsafe_copy_from_user() implementation
> 
> This commit introduces the unsafe_copy_from_user() macro for x86 architectures.
> It also refactors the existing unsafe_copy_to_user() macro to use updated
> internal variable names.
> 
> Regarding the variable naming in the updated unsafe_copy_to_user() and the
> new unsafe_copy_from_user() macros, is there a risk of variable shadowing?
> 
> The macros declare local variables with very common names, specifically __dst,
> __src, and __len. If a caller passes an argument that happens to share one of
> these names, the local variable declaration will shadow the caller's argument.
> For instance, if a caller passes __len, the initialization size_t __len =
> (_len); will evaluate the right-hand side as the newly declared uninitialized
> local variable itself. Could this lead to the loop copying a random length of
> memory based on stack garbage instead of the intended length? The previous
> implementation seemed to avoid this by using uniquely prefixed names like
> __ucu_len.

Please advise.

> Additionally, looking at the unsafe_copy_from_user() macro, the user-space
> source pointer is initialized as a void pointer without a const qualifier.
> 
> Since memory read operations typically expect the source buffer to be
> read-only, callers might pass a const void pointer. Does this implicit
> dropping of the const qualifier trigger a discarded-qualifiers compiler
> warning? Could the source pointer be declared as a const void pointer
> to prevent potential build failures when warnings are treated as errors?

Makes sense.
Thanks and 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/