Re: [PATCH] fortify: Avoid shadowing previous locals

[Kees Cook <keescook@chromium.org>]

On Mon, Oct 25, 2021 at 04:15:28PM -0400, Qian Cai wrote:
> 
> 
> On 10/25/21 3:34 PM, Kees Cook wrote:
> > On Mon, Oct 25, 2021 at 02:37:28PM -0400, Qian Cai wrote:
> >> __compiletime_strlen macro expansion will shadow p_size and p_len local
> >> variables. Just rename those in __compiletime_strlen.
> > 
> > They don't escape their local context, though, right? i.e. I don't see a
> > problem with the existing macro. Did you encounter a specific issue that
> > this patch fixes?
> 
> Yes, this is pretty minor. There are also some extra compiling warnings (W=2)
> from it.
> 
> ./include/linux/fortify-string.h: In function 'strnlen':
> 
> ./include/linux/fortify-string.h:17:9: warning: declaration of 'p_size' shadows a previous local [-Wshadow]
> 
>    17 |  size_t p_size = __builtin_object_size(p, 1); \
> 
>       |         ^~~~~~
> 
> ./include/linux/fortify-string.h:77:17: note: in expansion of macro '__compiletime_strlen'
>    77 |  size_t p_len = __compiletime_strlen(p);
> 
>       |                 ^~~~~~~~~~~~~~~~~~~~
> 
> ./include/linux/fortify-string.h:76:9: note: shadowed declaration is here
> 
>    76 |  size_t p_size = __builtin_object_size(p, 1);
> 
>       |         ^~~~~~

Gotcha. Yeah, we have -Wshadow=local tracked here:
https://github.com/KSPP/linux/issues/152

The changes needed to fix __wait_event() are extensive, though. But yes,
there's no good reason for this macro to make things worse for W=2. ;)

I'd like to keep the existing names, so many just prefixing them with
"__" and send a v2?

Thanks!

-- 
Kees Cook