linux-kernel - RE: [PATCH 1/4] Adds -Wshadow=local on KBUILD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 19 Oct 2018 11:28:19 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Masahiro Yamada' <yamada.masahiro@...ionext.com>,
        Borislav Petkov <bp@...en8.de>
CC:     Leonardo Brás <leobras.c@...il.com>,
        "lkcamp@...ts.libreplanetbr.org" <lkcamp@...ts.libreplanetbr.org>,
        "Matthew Wilcox" <willy6545@...il.com>,
        Andy Lutomirski <luto@...nel.org>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, X86 ML <x86@...nel.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>
Subject: RE: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS

From: Masahiro Yamada
> Sent: 18 October 2018 17:39
> 
> On Thu, Oct 18, 2018 at 6:18 PM Borislav Petkov <bp@...en8.de> wrote:
> >
> > On Wed, Oct 17, 2018 at 09:40:53PM -0300, Leonardo Bras wrote:
> > > The idea was to put it as default and fix all the shadowing warnings.
> > > What do you think?  I am open to suggestions.
> >
> > That's Masahiro's call. In the rest of the kernel, those warnings are behind
> > the W=2 switch - i.e., not enabled by default.
> 
> 
> It is not realistic to enable this warning option by default.
> Even -Wshadow=local emits tons of warnings.
> (More with -Wshadow)

The question is, how many of them are actual bugs.
IMHO -Wshadow is a good idea.

> The problem of this flag is,
> it is false positive in macro expansions.

Right, but macro expansions inside macro definitions could accidentally
use the same local variable - leading to choas.

> For example, I think the following is a legitimate case.
...
> arch/arm64/kernel/fpsimd.c:713:2: note: in expansion of macro ‘write_sysreg’
>   write_sysreg(read_sysreg(CPACR_EL1) | CPACR_EL1_ZEN_EL1EN, CPACR_EL1);
>   ^~~~~~~~~~~~

Easily fixed by using different named temporaries in the two macros.
There probably aren't that many macro pairs where that happens.
Especially since many are now inlined functions.

It might be that a small number of changes get rid of most of the warnings.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)