Message-ID: <CAK8P3a2ijLz0mtW5j9VamuQLdCOyLsr_wTVysi=X_XZyMGAOGw@mail.gmail.com>
Date:   Wed, 27 Nov 2019 21:07:51 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Kees Cook <keescook@...omium.org>
Cc:     "Shiyunming (Seth, RTOS)" <shiyunming@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "lizi (D)" <lizi4@...wei.com>, "Sunke (SK)" <sunke09@...wei.com>,
        Jiangyangyang <jiangyangyang@...wei.com>,
        Linzichang <linzichang@...wei.com>,
        Kernel Hardening <kernel-hardening@...ts.openwall.com>,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>
Subject: Re: Questions about "security functions" and "suppression of
 compilation alarms".

On Wed, Nov 27, 2019 at 8:01 PM Kees Cook <keescook@...omium.org> wrote:
> On Wed, Nov 27, 2019 at 01:11:50PM +0000, Shiyunming (Seth, RTOS) wrote:
> Each of these needs to be handled on a case-by-case basis. Kernel builds
> are expected to build without warnings, so before a new compiler flag
> can be added to the global build, all the instances need to be
> addressed. (Flags are regularly turned off because they are enabled by
> default in newer compiler versions but this causes too many warnings.)
> See the "W=1", "W=2", etc build options for enabling these:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/Makefile.extrawarn
>
> Once all instances of a warning are eliminated, these warnings can be
> moved to the top-level Makefile. Arnd Bergmann does a lot of this work
> and might be able to speak more coherently about this. :) For example,
> here is enabling of -Wmaybe-uninitialized back in the v4.10 kernel:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4324cb23f4569edcf76e637cdb3c1dfe8e8a85e4

Incidentally, I've worked on changing the way we handle warnings over
the past week, going through all ~700 warning options supported by
either clang or gcc to figure out how many instances we get and if
we are missing any important ones.

https://pastebin.com/wqG9QgHj has a list of the warnings that exist,
when they got added and how many I saw. I'll post this as proper
patches when I have integrated this into the build system better.

> Speaking specifically to your list:
>
> -Wformat-security
>   This has tons of false positives, and likely requires fixing the
>   compiler.

the only warning I see here is "warning: format not a string literal and
no format arguments", this seems useful at the W=1 or W=2
level, but I can't even think of how to change the compiler to
turn this on by default.

> -Wpointer-sign
>   Lots of things in the kernel pass pointers around in weird ways. This
>   is disabled to allow normal operation (which, combined with
>   -fwrapv-pointer and -fwrapv via -fno-strict-overflow) means signed
>   things and pointers behave without "undefined behavior". A lot of work
>   would be needed all over the kernel to enable this warning (and part
>   of that would be incrementally removing unexpected overflows of both
>   unsigned and signed arithmetic).

I have the suspicion that this would actually find some serious bugs,
but I also share your view that this is near-impossible to fix
throughout the kernel.

My experiments show around 3000 files that cause this warning,
though fixing the ones in shared header files would get us closer
to enabling it at W=1. Most of the output from this seems to be from
two header files.

> -Wframe-address
>   __builtin_frame_address() gets used in "safe" places on the
>   architectures where the limitations are understood, so adding this
>   warning doesn't gain anything because it's already rare and gets
>   some scrutiny.

This one could be enabled by default and then disabled locally
in functions that are known to be safe.

> -Wmaybe-uninitialized
>   And linked above, this is enabled by default since v4.10.
>
> -Wformat-overflow
>   See https://git.kernel.org/linus/bd664f6b3e376a8ef4990f87d08271cc2d01ba9a
>   for details. Eliminating these warnings (there were 1500) needs to
>   happen before they can be turned back on. Any help here is very
>   welcome!

In the current kernel, I see only around 100 files that produce this warning,
so this one is definitely in reach.

> Warnings seen from newly introduced code get fixed very quickly, yes.
> Problems that were already existing and are surfaced by new warnings
> tend to get less direct attention by maintainers since it creates a
> large amount of work where it is hard to measure the benefit. However,
> people contributing changes in these areas tend to be very well received.
> For example, Gustavo A. R. Silva did a huge amount of work to enable
> -Wimplicit-fallthrough: https://lwn.net/Articles/794944/

With the patch series I'm working on, we will be able to control the warning
level (W=1, W=12 etc) per file and per subsystem, which I hope should make
it easier to attack some of the hard problems by fixing a whole class
of warnings one subsystem at a time.

     Arnd

Powered by blists - more mailing lists