linux-kernel - Re: [PATCH] Kbuild: Move -Wmaybe-uninitialized to W=1

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]

Message-ID: <1404811520.26126.38.camel@x220>
Date:	Tue, 08 Jul 2014 11:25:20 +0200
From:	Paul Bolle <pebolle@...cali.nl>
To:	Borislav Petkov <bp@...en8.de>
Cc:	Sam Ravnborg <sam@...nborg.org>,
	lkml <linux-kernel@...r.kernel.org>, Michael Matz <matz@...e.de>,
	linux-kbuild@...r.kernel.org, x86-ml <x86@...nel.org>
Subject: Re: [PATCH] Kbuild: Move -Wmaybe-uninitialized to W=1

On Mon, 2014-07-07 at 12:53 +0200, Borislav Petkov wrote:
> This warning is enabled by -Wall or -Wextra, says the gcc manpage. It
> also says that gcc cannot always know whether the warning is issued
> correctly:
> 
> "These warnings are made optional because GCC is not smart enough to see
> all the reasons why the code might be correct in spite of appearing to
> have an error."
> 
> And, as expected, it fires for perfectly valid use cases, thus making it
> not really useful. Let's move it to the W=1 bunch in case people want to
> enable it with the additional checks.
> 
> Signed-off-by: Borislav Petkov <bp@...e.de>

Is the fact that this generates false positives by itself enough to
downgrade this check to W=1?

I do not have any hard numbers to back up my claims, but I'd like to
point out that it is possible that we never see many of the warnings
that GCC correctly issues because they might only occur during local
development. Ie, the developer involved cleans up the patch before
sending out.

My experience with the warnings that actually do make it into mainline
is that quite a few are correct while the false positives tend to be
generated by a pieces of code that are complicated (I think I've seen
the warning labeled as a "code smell").

For example, in my local builds this warning popped up three times in
the current development cycle:
    0) fs/direct-io.c:1011:12: warning: ‘from’ may be used uninitialized in this function [-Wmaybe-uninitialized]
       fs/direct-io.c:1011:12: warning: ‘to’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    1) drivers/platform/x86/eeepc-laptop.c:279:10: warning: ‘value’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    2) drivers/usb/misc/usb3503.c:195:11: warning: ‘err’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Warning 0) occurs in a 150 line function, that grows over 200 lines when
the inline functions it calls are included. And I do think it's not easy
to see hwat that code does. Anyhow, see
https://lkml.org/lkml/2014/7/1/496 for my attempt to silence this
warning by simplifying this function.

See https://lkml.org/lkml/2014/7/1/150 for my patch that silences 1) by,
again, simplifying the code.

And warning 2) is correct. See https://lkml.org/lkml/2014/6/2/511 for a
possible solution.

So, in summary, I believe that the signal/noise ratio actually isn't too
bad. Moreover I think that the noise isn't merely noise, as it points to
possibly problematic sections of code.

Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/