Re: [PATCH] crypto: testmgr - reduce stack usage in fuzzers

[Arnd Bergmann]

On Mon, Jun 17, 2019 at 4:04 PM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Jun 17, 2019 at 03:23:02PM +0200, Arnd Bergmann wrote:
> > On arm32, we get warnings about high stack usage in some of the functions:
> >
> > crypto/testmgr.c:2269:12: error: stack frame size of 1032 bytes in function 'alg_test_aead' [-Werror,-Wframe-larger-than=]
> > static int alg_test_aead(const struct alg_test_desc *desc, const char *driver,
> >            ^
> > crypto/testmgr.c:1693:12: error: stack frame size of 1312 bytes in function '__alg_test_hash' [-Werror,-Wframe-larger-than=]
> > static int __alg_test_hash(const struct hash_testvec *vecs,
> >            ^
> >
> > On of the larger objects on the stack here is struct testvec_config, so
> > change that to dynamic allocation.
> >
> > Fixes: 40153b10d91c ("crypto: testmgr - fuzz AEADs against their generic implementation")
> > Fixes: d435e10e67be ("crypto: testmgr - fuzz skciphers against their generic implementation")
> > Fixes: 9a8a6b3f0950 ("crypto: testmgr - fuzz hashes against their generic implementation")
> > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> > ---
> > I only compile-tested this, and it's not completely trivial, so please
> > review carefully.
>
> These structures are not meant to be that big.  I suspect something
> has gone awry with the recent security conversions.
>
> Kees?

I should have mentioned above that this happened with clang but not gcc.

We used to not be able to test with clang and KASAN. I had done some of
those tests in the past, but that was before Kees' nice cleanup, so the
potential stack overflow would already happen but not detected by the
compiler.

Both gcc and clang add a redzone around each stack variable that gets
passed into an 'extern' variable. The difference here is that with clang, the
size of that redzone is proportional to the size of the object, while with gcc
it is constant.

In most cases, this ends up in favor of clang (concerning the stack
warning size limit) because most variables are small, but here we have
a large stack object (two objects for the hash fuzzing) with a large redzone.

         Arnd