linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Kees Cook <keescook@chromium.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	Ard Biesheuvel <ardb@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	kernel test robot <lkp@intel.com>,
	Peter Oberparleiter <oberpar@linux.ibm.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	kbuild-all@lists.01.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>
Subject: Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes
Date: Thu, 27 Aug 2020 12:02:12 -0700	[thread overview]
Message-ID: <CAHk-=wjPasyJrDuwDnpHJS2TuQfExwe=px-SzLeN8GFMAQJPmQ@mail.gmail.com> (raw)
In-Reply-To: <202008271138.0FA7400@keescook>

On Thu, Aug 27, 2020 at 11:42 AM Kees Cook <keescook@chromium.org> wrote:
>
> Do you mean you checked both gcc and clang and it was only a problem with gcc?

I didn't check with clang, but Arnd claimed it was fine.

> (If so, I can tweak the "depends" below...)

Ugh.

Instead of making the Makefile even uglier, why don't you just make
this all be done in the Kconfig.

Also, I'm not seeing the point of your patch. You didn't actually
change anything, you just made a new config variable with the same
semantics as the old one.

Add a

        depends on CLANG

or something, with a comment saying that it doesn't work on gcc due to
excessive stack use.

> +ifdef CONFIG_UBSAN_OBJECT_SIZE
> +      CFLAGS_UBSAN += $(call cc-option, -fsanitize=object-size)
> +endif

All of this should be thrown out, and this code should use the proper
patterns for configuration entries in the Makefile, ie just

  ubsan-cflags-$(CONFIG_UBSAN_OBJECT_SIZE) += -fsanitize=object-size

and the Kconfig file is the thing that should check if that CC option
exists with

  config UBSAN_OBJECT_SIZE
        bool "Check for accesses beyond known object sizes"
        default UBSAN
        depends on CLANG  # gcc makes a mess of it
        depends on $(cc-option,-fsanitize-coverage=trace-pc)

and the same goes for all the other cases too:

>  ifdef CONFIG_UBSAN_MISC
>        CFLAGS_UBSAN += $(call cc-option, -fsanitize=shift)
>        CFLAGS_UBSAN += $(call cc-option, -fsanitize=integer-divide-by-zero)
>        CFLAGS_UBSAN += $(call cc-option, -fsanitize=unreachable)
>        CFLAGS_UBSAN += $(call cc-option, -fsanitize=signed-integer-overflow)
> -      CFLAGS_UBSAN += $(call cc-option, -fsanitize=object-size)
>        CFLAGS_UBSAN += $(call cc-option, -fsanitize=bool)
>        CFLAGS_UBSAN += $(call cc-option, -fsanitize=enum)
>  endif

and if you don't want to ask for them (which is a good idea), you keep that

    config UBSAN_MISC
        bool "Misc UBSAN.."

thing, and just make all of the above have the pattern of

    config UBSAN_OBJECT_SIZE
        def_bool UBSAN_MISC
        depends on CLANG  # gcc makes a mess of it
        depends on $(cc-option,-fsanitize-coverage=trace-pc)

which makes the Makefile much cleaner, and makes all our choices very
visible in the config file when they then get passed around.

We should basically strive for our Makefiles to have as little "ifdef"
etc magic as possible. We did the config work already, the Makefiles
should primarily just have those

   XYZ-$(CONFIG_OPTION) += abc

kind of lines (and then  you often end up having

  CFLAGS_UBSAN := $(ubsan-cflags-y)

at the end).

Doesn't that all look much cleaner?

                   Linus

  reply	other threads:[~2020-08-27 19:02 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-27  3:52 kernel test robot
2020-08-27  8:05 ` Herbert Xu
2020-08-27  8:10   ` Ard Biesheuvel
2020-08-27  8:24     ` Herbert Xu
2020-08-27 17:34       ` Linus Torvalds
2020-08-27 17:55         ` Linus Torvalds
2020-08-27 18:42           ` Kees Cook
2020-08-27 19:02             ` Linus Torvalds [this message]
2020-08-27 19:32               ` Kees Cook
2020-08-27 19:11           ` Arnd Bergmann
2020-08-27 19:34             ` Kees Cook
2020-08-27 20:08             ` Linus Torvalds
2020-08-27  8:33     ` Arnd Bergmann
2020-08-27  8:42       ` Ard Biesheuvel
2020-08-27  9:19         ` Arnd Bergmann
2020-08-27 10:41           ` Ard Biesheuvel
2020-08-27 11:51             ` Herbert Xu
2020-08-27 16:25               ` Arnd Bergmann
2020-09-19 17:27 kernel test robot
2020-10-18 19:13 kernel test robot
2020-10-19 15:47 ` Joe Perches
2020-10-20  8:00   ` David Laight
2020-10-20 10:13     ` Joe Perches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wjPasyJrDuwDnpHJS2TuQfExwe=px-SzLeN8GFMAQJPmQ@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=ardb@kernel.org \
    --cc=arnd@arndb.de \
    --cc=aryabinin@virtuozzo.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=kbuild-all@lists.01.org \
    --cc=keescook@chromium.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=oberpar@linux.ibm.com \
    --subject='Re: lib/crypto/chacha.c:65:1: warning: the frame size of 1604 bytes is larger than 1024 bytes' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).