On Wed, Sep 26, 2018 at 2:51 AM, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: > Arnd reports that with Kees's latest VLA patches applied, the HMAC > handling in the QAT driver uses a worst case estimate of 160 bytes > for the SHA blocksize, allowing the compiler to determine the size > of the stack frame at runtime and throw a warning: > > drivers/crypto/qat/qat_common/qat_algs.c: In function 'qat_alg_do_precomputes': > drivers/crypto/qat/qat_common/qat_algs.c:257:1: error: the frame size > of 1112 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > Given that this worst case estimate is only 32 bytes larger than the > actual block size of SHA-512, the use of a VLA here was hiding the > excessive size of the stack frame from the compiler, and so we should > try to move these buffers off the stack. > > So move the ipad/opad buffers and the various SHA state descriptors > into the tfm context struct. Since qat_alg_do_precomputes() is only > called in the context of a setkey() operation, this should be safe. > Using SHA512_BLOCK_SIZE for the size of the ipad/opad buffers allows > them to be used by SHA-1/SHA-256 as well. > > Reported-by: Arnd Bergmann <arnd@xxxxxxxx> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> > --- > This applies against v4.19-rc while Arnd's report was about -next. However, > since Kees's VLA change results in a warning about a pre-existing condition, > we may decide to apply it as a fix, and handle the conflict with Kees's > patch in cryptodev. Otherwise, I can respin it to apply onto cryptodev > directly. The patch was build tested only - I don't have the hardware. I think the depth warning is minor (90 bytes over), so I don't think it's high priority to backport the fix. I'm fine either way, of course. Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx> -Kees -- Kees Cook Pixel Security
