Re: [PATCH net-next v5 02/20] zinc: introduce minimal cryptography library
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: 2018-09-21 00:12:02
Also in:
linux-crypto, lkml
Hey Arnd, On Thu, Sep 20, 2018 at 6:02 PM Arnd Bergmann [off-list ref] 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