linux-kernel - Re: [PATCH net-next v5 02/20] zinc: introduce minimal cryptography library

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: <CAHmME9rJq=q7xj_E_84GN5FuzSW2KdP4Ch=y527Lem0ME4GnLg@mail.gmail.com>
Date:   Fri, 21 Sep 2018 02:11:43 +0200
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Eric Biggers <ebiggers@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Netdev <netdev@...r.kernel.org>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Samuel Neves <sneves@....uc.pt>,
        Andrew Lutomirski <luto@...nel.org>,
        Jean-Philippe Aumasson <jeanphilippe.aumasson@...il.com>
Subject: Re: [PATCH net-next v5 02/20] zinc: introduce minimal cryptography library

Hey Arnd,

On Thu, Sep 20, 2018 at 6:02 PM Arnd Bergmann <arnd@...db.de> wrote:
> Right, if you hit a stack requirement like this, it's usually the compiler
> doing something bad, not just using too much stack but also generating
> rather slow object code in the process. It's better to fix the bug by
> optimizing the code to not spill registers to the stack.
>
> In the long run, I'd like to reduce the stack frame size further, so
> best assume that anything over 1024 bytes (on 32-bit) or 1280 bytes
> (on 64-bit) is a bug in the code, and stay below that.
>
> For prototyping, you can just mark the broken functions individually
> by setting the warning limit for a specific function that is known to
> be misoptimized by the compiler (with a comment about which compiler
> and architectures are affected), but not override the limit for the
> entire file.

Thanks for the explanation. Fortunately in my case, the issues were
trivially fixable to get it under 1024/1280. (By the way, why does
64-bit have a slightly larger stack frame? To account for 32 pointers
taking double the space or something?) That will be rectified in v6.

There is one exception though: sometimes KASAN bloats the frame on
64-bit compiles. How would you feel about me adding
'ccflags-$(CONFIG_KASAN) += -Wframe-larger-than=16384' in my makefile?
I'm not remotely close to reaching that limit (it's just a tiny bit
over 1280), but it does seem like telling gcc to quiet down about
stack frames when KASAN is being used might make sense. Alternatively,
I see the defaults for FRAME_WARN are:

config FRAME_WARN
       int "Warn for stack frames larger than (needs gcc 4.4)"
       range 0 8192
       default 3072 if KASAN_EXTRA
       default 2048 if GCC_PLUGIN_LATENT_ENTROPY
       default 1280 if (!64BIT && PARISC)
       default 1024 if (!64BIT && !PARISC)
       default 2048 if 64BIT

What about changing that KASAN_EXTRA into just KASAN? This seems
cleanest; I'll send a patch for it.

On the other hand, this KASAN behavior is only observable on 64-bit
systems when I force it to be 1280, whereas the default is still 2048,
so probably this isn't a problem *now* for this patchset. But it is
something to think about for down the road when you lower the default
frame.

Regards,
Jason