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

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]

Date:   Mon, 17 Jun 2019 16:10:44 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Herbert Xu <herbert@...dor.apana.org.au>
Cc:     Kees Cook <keescook@...omium.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Biggers <ebiggers@...gle.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Vitaly Chikunov <vt@...linux.org>,
        Gilad Ben-Yossef <gilad@...yossef.com>,
        "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] crypto: testmgr - reduce stack usage in fuzzers

On Mon, Jun 17, 2019 at 4:04 PM Herbert Xu <herbert@...dor.apana.org.au> 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@...db.de>
> > ---
> > 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