On Mon, Jun 25, 2018 at 02:10:25PM -0700, Kees Cook wrote: > In the quest to remove all stack VLA usage from the kernel[1], this caps > the ahash request size similar to the other limits and adds a sanity > check at registration. Unfortunately, these reqsizes can be huge. Looking > at all callers of crypto_ahash_set_reqsize(), the largest appears to be > 664 bytes, based on a combination of manual inspection and pahole output: > > 4 dcp_sha_req_ctx > 40 crypto4xx_ctx > 52 hash_req_ctx > 80 ahash_request > 88 rk_ahash_rctx > 104 sun4i_req_ctx > 200 mcryptd_hash_request_ctx > 216 safexcel_ahash_req > 228 sha1_hash_ctx > 228 sha256_hash_ctx > 248 img_hash_request_ctx > 252 mtk_sha_reqctx > 276 sahara_sha_reqctx > 304 mv_cesa_ahash_req > 316 iproc_reqctx_s > 320 caam_hash_state > 320 qce_sha_reqctx > 356 sha512_hash_ctx > 384 ahash_req_ctx > 400 chcr_ahash_req_ctx > 416 stm32_hash_request_ctx > 432 talitos_ahash_req_ctx > 462 atmel_sha_reqctx > 496 ccp_aes_cmac_req_ctx > 616 ccp_sha_req_ctx > 664 artpec6_hash_request_context > > So, this chooses 720 as a larger "round" number for the max. > > [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@xxxxxxxxxxxxxx > > Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> > Cc: "David S. Miller" <davem@xxxxxxxxxxxxx> > Cc: Eric Biggers <ebiggers@xxxxxxxxxx> > Cc: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx> > Cc: Rabin Vincent <rabinv@xxxxxxxx> > Cc: Lars Persson <larper@xxxxxxxx> > Cc: linux-crypto@xxxxxxxxxxxxxxx > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> > --- > include/crypto/hash.h | 3 ++- > include/crypto/internal/hash.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/crypto/hash.h b/include/crypto/hash.h > index 5d79e2f0936e..b550077c4767 100644 > --- a/include/crypto/hash.h > +++ b/include/crypto/hash.h > @@ -66,10 +66,11 @@ struct ahash_request { > > #define AHASH_MAX_DIGESTSIZE 512 > #define AHASH_MAX_STATESIZE 512 > +#define AHASH_MAX_REQSIZE 720 > > #define AHASH_REQUEST_ON_STACK(name, ahash) \ > char __##name##_desc[sizeof(struct ahash_request) + \ > - crypto_ahash_reqsize(ahash)] CRYPTO_MINALIGN_ATTR; \ > + AHASH_MAX_REQSIZE] CRYPTO_MINALIGN_ATTR; \ > struct ahash_request *name = (void *)__##name##_desc > > /** > diff --git a/include/crypto/internal/hash.h b/include/crypto/internal/hash.h > index a0b0ad9d585e..d96ae5f52125 100644 > --- a/include/crypto/internal/hash.h > +++ b/include/crypto/internal/hash.h > @@ -142,6 +142,7 @@ static inline struct ahash_alg *__crypto_ahash_alg(struct crypto_alg *alg) > static inline void crypto_ahash_set_reqsize(struct crypto_ahash *tfm, > unsigned int reqsize) > { > + BUG_ON(reqsize > AHASH_MAX_REQSIZE); > tfm->reqsize = reqsize; > } This isn't accounting for the cases where a hash algorithm is "wrapped" with another one, which increases the request size. For example, "sha512_mb" ends up with a request size of sizeof(struct ahash_request) + sizeof(struct mcryptd_hash_request_ctx) + sizeof(struct ahash_request) + sizeof(struct sha512_hash_ctx) == 808 bytes, on x86_64 with CONFIG_DEBUG_SG enabled. (Note also that structure sizes can vary depending on the architecture and the kernel config.) So, with the self-tests enabled your new BUG_ON() is hit on boot: ------------[ cut here ]------------ kernel BUG at ./include/crypto/internal/hash.h:145! invalid opcode: 0000 [#1] SMP PTI CPU: 4 PID: 337 Comm: cryptomgr_test Not tainted 4.18.0-rc2-00048-gda396e1e8ca5 #11 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014 RIP: 0010:mcryptd_hash_init_tfm+0x36/0x40 crypto/mcryptd.c:289 Code: 01 00 00 e8 0c 05 fd ff 48 3d 00 f0 ff ff 77 18 48 89 43 40 8b 40 40 05 c8 00 00 00 3d d0 02 00 00 77 07 89 43 f8 31 c0 5b c3 <0f> 0b 0f 1f 84 00 00 00 00 00 80 7f 0c 00 74 01 c3 65 8b 05 d2 e2 RSP: 0000:ffffb180c0dafc50 EFLAGS: 00010202 RAX: 00000000000002d8 RBX: ffffa1867c9267c8 RCX: ffffffffb66f5ef0 RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffa1867c927c48 RBP: ffffa1867c926780 R08: 0000000000000001 R09: ffffa1867c927c00 R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 R13: ffffa1867c9c6848 R14: ffffa1867c9c6848 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffffa1867fd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000035c10001 CR4: 00000000001606e0 Call Trace: crypto_create_tfm+0x86/0xd0 crypto/api.c:475 crypto_alloc_tfm+0x4b/0xb0 crypto/api.c:543 mcryptd_alloc_ahash+0x6f/0xb0 crypto/mcryptd.c:603 sha512_mb_async_init_tfm+0x1a/0x50 arch/x86/crypto/sha512-mb/sha512_mb.c:724 crypto_create_tfm+0x86/0xd0 crypto/api.c:475 crypto_alloc_tfm+0x4b/0xb0 crypto/api.c:543 __alg_test_hash+0x1c/0x90 crypto/testmgr.c:1799 alg_test_hash+0x6b/0x100 crypto/testmgr.c:1841 alg_test.part.5+0x119/0x2a0 crypto/testmgr.c:3687 cryptomgr_test+0x3b/0x40 crypto/algboss.c:223 kthread+0x114/0x130 kernel/kthread.c:240 ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412 Modules linked in: ---[ end trace d726be03a53bddb5 ]---
