Re: Questions about "security functions" and "suppression of compilation alarms".

Re: Questions about "security functions" and "suppression of compilation alarms".

[Kees Cook]

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

Re: Questions about "security functions" and "suppression of compilation alarms".

[Arnd Bergmann]

On Wed, Nov 27, 2019 at 8:01 PM Kees Cook <keescook@chromium.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