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

[Arnd Bergmann]

On Mon, Jun 17, 2019 at 4:56 PM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> 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