On 5 October 2018 at 04:29, Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote: > On Wed, Sep 26, 2018 at 11:51:59AM +0200, Ard Biesheuvel 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. >> >> Thoughts anyone? > > I applied it against cryptodev only. I don't think it's bad enough > to warrant a backport to stable though. But if you guys disagree we > could always send the backport to stable after this goes upstream. > Works for me.
