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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a1e4RGZGD3Ww8ym5iXdt7v8GPxY29jywNnZ-Wx51L-beQ@mail.gmail.com>
Date:   Mon, 17 Jun 2019 17:22:47 +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:56 PM Herbert Xu <herbert@...dor.apana.org.au> wrote:
>
> On Mon, Jun 17, 2019 at 04:54:16PM +0200, Arnd Bergmann wrote:
> >
> > Just converting the three testvec_config variables is what I originally
> > had in my patch. It got some configurations below the warning level,
> > but some others still had the problem. I considered sending two
> > separate patches, but as the symptom was the same, I just folded
> > it all into one patch that does the same thing in four functions.
>
> Just curious, how bad is it with only moving testvec_config off
> the stack?

I tried setting the warning limit to 256 now. On the original code I get
crypto/testmgr.c:2816:12: error: stack frame size of 984 bytes in
function 'alg_test_skcipher'
crypto/testmgr.c:2273:12: error: stack frame size of 1032 bytes in
function 'alg_test_aead'
crypto/testmgr.c:3267:12: error: stack frame size of 576 bytes in
function 'alg_test_crc32c'
crypto/testmgr.c:3811:12: error: stack frame size of 280 bytes in
function 'alg_test_akcipher'
crypto/testmgr.c:2798:12: error: stack frame size of 600 bytes in
function 'test_skcipher'
crypto/testmgr.c:2413:12: error: stack frame size of 352 bytes in
function 'test_skcipher_vec_cfg'
crypto/testmgr.c:2255:12: error: stack frame size of 600 bytes in
function 'test_aead'
crypto/testmgr.c:1823:12: error: stack frame size of 368 bytes in
function 'test_aead_vec_cfg'
crypto/testmgr.c:1694:12: error: stack frame size of 1408 bytes in
function '__alg_test_hash'

Just removing the testvec_config reduces the size of the largest three functions
by some 350 bytes, but I still get a warning for __alg_test_hash in some
configurations with the default 1024 byte limit:

crypto/testmgr.c:2837:12: error: stack frame size of 632 bytes in
function 'alg_test_skcipher'
crypto/testmgr.c:2287:12: error: stack frame size of 688 bytes in
function 'alg_test_aead'
crypto/testmgr.c:3288:12: error: stack frame size of 576 bytes in
function 'alg_test_crc32c'
crypto/testmgr.c:3832:12: error: stack frame size of 280 bytes in
function 'alg_test_akcipher'
crypto/testmgr.c:2819:12: error: stack frame size of 600 bytes in
function 'test_skcipher'
crypto/testmgr.c:2427:12: error: stack frame size of 352 bytes in
function 'test_skcipher_vec_cfg'
crypto/testmgr.c:2269:12: error: stack frame size of 600 bytes in
function 'test_aead'
crypto/testmgr.c:1830:12: error: stack frame size of 368 bytes in
function 'test_aead_vec_cfg'
crypto/testmgr.c:1701:12: error: stack frame size of 1088 bytes in
function '__alg_test_hash'

With the patch I posted, the last line goes down to 712:

crypto/testmgr.c:1709:12: error: stack frame size of 712 bytes in
function '__alg_test_hash'

In other subsystems, the numbers tend to be much smaller than in the crypto
code. I think a lot of that is inherent in the way the subsystem is designed,
but it also seems a little dangerous.

        Arnd