From cfa1d74495aa3cf240fd2b1fb45d43cc2a754a46 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Wed, 26 Sep 2018 11:51:59 +0200 Subject: crypto: qat - move temp buffers off the stack 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 compile time 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 Signed-off-by: Ard Biesheuvel Reviewed-by: Kees Cook Signed-off-by: Herbert Xu --- drivers/crypto/qat/qat_common/qat_algs.c | 64 ++++++++++++++++---------------- 1 file changed, 31 insertions(+), 33 deletions(-) (limited to 'drivers/crypto/qat') diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c index a28edf7b792f..d2698299896f 100644 --- a/drivers/crypto/qat/qat_common/qat_algs.c +++ b/drivers/crypto/qat/qat_common/qat_algs.c @@ -113,6 +113,13 @@ struct qat_alg_aead_ctx { struct crypto_shash *hash_tfm; enum icp_qat_hw_auth_algo qat_hash_alg; struct qat_crypto_instance *inst; + union { + struct sha1_state sha1; + struct sha256_state sha256; + struct sha512_state sha512; + }; + char ipad[SHA512_BLOCK_SIZE]; /* sufficient for SHA-1/SHA-256 as well */ + char opad[SHA512_BLOCK_SIZE]; }; struct qat_alg_ablkcipher_ctx { @@ -148,41 +155,32 @@ static int qat_alg_do_precomputes(struct icp_qat_hw_auth_algo_blk *hash, unsigned int auth_keylen) { SHASH_DESC_ON_STACK(shash, ctx->hash_tfm); - struct sha1_state sha1; - struct sha256_state sha256; - struct sha512_state sha512; int block_size = crypto_shash_blocksize(ctx->hash_tfm); int digest_size = crypto_shash_digestsize(ctx->hash_tfm); - char ipad[MAX_ALGAPI_BLOCKSIZE]; - char opad[MAX_ALGAPI_BLOCKSIZE]; __be32 *hash_state_out; __be64 *hash512_state_out; int i, offset; - memset(ipad, 0, block_size); - memset(opad, 0, block_size); + memset(ctx->ipad, 0, block_size); + memset(ctx->opad, 0, block_size); shash->tfm = ctx->hash_tfm; shash->flags = 0x0; - if (WARN_ON(block_size > sizeof(ipad) || - sizeof(ipad) != sizeof(opad))) - return -EINVAL; - if (auth_keylen > block_size) { int ret = crypto_shash_digest(shash, auth_key, - auth_keylen, ipad); + auth_keylen, ctx->ipad); if (ret) return ret; - memcpy(opad, ipad, digest_size); + memcpy(ctx->opad, ctx->ipad, digest_size); } else { - memcpy(ipad, auth_key, auth_keylen); - memcpy(opad, auth_key, auth_keylen); + memcpy(ctx->ipad, auth_key, auth_keylen); + memcpy(ctx->opad, auth_key, auth_keylen); } for (i = 0; i < block_size; i++) { - char *ipad_ptr = ipad + i; - char *opad_ptr = opad + i; + char *ipad_ptr = ctx->ipad + i; + char *opad_ptr = ctx->opad + i; *ipad_ptr ^= HMAC_IPAD_VALUE; *opad_ptr ^= HMAC_OPAD_VALUE; } @@ -190,7 +188,7 @@ static int qat_alg_do_precomputes(struct icp_qat_hw_auth_algo_blk *hash, if (crypto_shash_init(shash)) return -EFAULT; - if (crypto_shash_update(shash, ipad, block_size)) + if (crypto_shash_update(shash, ctx->ipad, block_size)) return -EFAULT; hash_state_out = (__be32 *)hash->sha.state1; @@ -198,22 +196,22 @@ static int qat_alg_do_precomputes(struct icp_qat_hw_auth_algo_blk *hash, switch (ctx->qat_hash_alg) { case ICP_QAT_HW_AUTH_ALGO_SHA1: - if (crypto_shash_export(shash, &sha1)) + if (crypto_shash_export(shash, &ctx->sha1)) return -EFAULT; for (i = 0; i < digest_size >> 2; i++, hash_state_out++) - *hash_state_out = cpu_to_be32(*(sha1.state + i)); + *hash_state_out = cpu_to_be32(ctx->sha1.state[i]); break; case ICP_QAT_HW_AUTH_ALGO_SHA256: - if (crypto_shash_export(shash, &sha256)) + if (crypto_shash_export(shash, &ctx->sha256)) return -EFAULT; for (i = 0; i < digest_size >> 2; i++, hash_state_out++) - *hash_state_out = cpu_to_be32(*(sha256.state + i)); + *hash_state_out = cpu_to_be32(ctx->sha256.state[i]); break; case ICP_QAT_HW_AUTH_ALGO_SHA512: - if (crypto_shash_export(shash, &sha512)) + if (crypto_shash_export(shash, &ctx->sha512)) return -EFAULT; for (i = 0; i < digest_size >> 3; i++, hash512_state_out++) - *hash512_state_out = cpu_to_be64(*(sha512.state + i)); + *hash512_state_out = cpu_to_be64(ctx->sha512.state[i]); break; default: return -EFAULT; @@ -222,7 +220,7 @@ static int qat_alg_do_precomputes(struct icp_qat_hw_auth_algo_blk *hash, if (crypto_shash_init(shash)) return -EFAULT; - if (crypto_shash_update(shash, opad, block_size)) + if (crypto_shash_update(shash, ctx->opad, block_size)) return -EFAULT; offset = round_up(qat_get_inter_state_size(ctx->qat_hash_alg), 8); @@ -231,28 +229,28 @@ static int qat_alg_do_precomputes(struct icp_qat_hw_auth_algo_blk *hash, switch (ctx->qat_hash_alg) { case ICP_QAT_HW_AUTH_ALGO_SHA1: - if (crypto_shash_export(shash, &sha1)) + if (crypto_shash_export(shash, &ctx->sha1)) return -EFAULT; for (i = 0; i < digest_size >> 2; i++, hash_state_out++) - *hash_state_out = cpu_to_be32(*(sha1.state + i)); + *hash_state_out = cpu_to_be32(ctx->sha1.state[i]); break; case ICP_QAT_HW_AUTH_ALGO_SHA256: - if (crypto_shash_export(shash, &sha256)) + if (crypto_shash_export(shash, &ctx->sha256)) return -EFAULT; for (i = 0; i < digest_size >> 2; i++, hash_state_out++) - *hash_state_out = cpu_to_be32(*(sha256.state + i)); + *hash_state_out = cpu_to_be32(ctx->sha256.state[i]); break; case ICP_QAT_HW_AUTH_ALGO_SHA512: - if (crypto_shash_export(shash, &sha512)) + if (crypto_shash_export(shash, &ctx->sha512)) return -EFAULT; for (i = 0; i < digest_size >> 3; i++, hash512_state_out++) - *hash512_state_out = cpu_to_be64(*(sha512.state + i)); + *hash512_state_out = cpu_to_be64(ctx->sha512.state[i]); break; default: return -EFAULT; } - memzero_explicit(ipad, block_size); - memzero_explicit(opad, block_size); + memzero_explicit(ctx->ipad, block_size); + memzero_explicit(ctx->opad, block_size); return 0; } -- cgit