Message-ID: <201911271013.38BA7015C6@keescook>
Date:   Wed, 27 Nov 2019 11:01:15 -0800
From:   Kees Cook <keescook@...omium.org>
To:     "Shiyunming (Seth, RTOS)" <shiyunming@...wei.com>
Cc:     "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@...ts.openwall.com, Arnd Bergmann <arnd@...db.de>,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>
Subject: Re: Questions about "security functions" and "suppression of
 compilation alarms".

On Wed, Nov 27, 2019 at 01:11:50PM +0000, Shiyunming (Seth, RTOS) wrote:
> During the use of Linux, I found lots of C standard functions such
> as memcpy and strcpy etc. These functions did not check the size of
> the target buffer and easily introduced the security vulnerability of
> buffer overflow.

See CONFIG_FORTIFY_SOURCE (which enables such bounds checking):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/string.h#n262
and the plans for improvement: https://github.com/KSPP/linux/issues/6

> And some compilation options are enabled to suppress compilation alarms,
> for example (Wno-format-security Wno-pointer-sign Wno-frame-address
> Wno-maybe-uninitialized Wno-format-overflow Wno-int-in-bool-context),
> which may bring potential security problems.

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

Speaking specifically to your list:

-Wformat-security
  This has tons of false positives, and likely requires fixing the
  compiler.

-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).

-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.

-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!

-Wint-in-bool-context
  See https://git.kernel.org/linus/a3bc88645e9293f5aaac9c05a185d9f1c0594c6c
  where it was enabled again in v5.2 after Arnd cleaned up the associated
  warnings.

> In response to these circumstances, my question is:
> (1) Does the kernel community think that using these functions and
> compiling alarm suppression have security problems?

Generally speaking, yes, of course, if we have tools that provide the
code base with better security (or more specifically, a reduction in all
bugs, not just those that may have security implications), we want to
use them. However, such things need to have a false positive rate that is
close to zero. If it has a high false positive rate, then there needs to
be a strong indication that the true positives are very serious problems
and some mechanism can be implemented to silence the false positives.

>     If it is considered as a problem, will the developer be promoted
> to fix it?

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/

>     If it is not considered as a problem, what is the reason?

Hopefully I've explained the nuances in this email. :)

> (2) The C11 specification contains security enhancement functions. What
> is the policy of the community about them? Is there any plan to use
> these functions?

Which do you mean specifically? Generally speaking, the community is
open to anything that can be reasonably maintained. :)

Are there any features you've tried to enable and you'd be interested in
submitting patches to fix?

Thanks for the questions!

-- 
Kees Cook

Powered by blists - more mailing lists