Messages in this thread | | | Date | Mon, 25 Oct 2021 13:49:57 -0700 | From | Kees Cook <> | Subject | Re: [PATCH] fortify: Avoid shadowing previous locals |
| |
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
| |