Re: [RFC PATCH 3/7] crypto: hctr2 - Add HCTR2 support

[Eric Biggers <ebiggers@kernel.org>]

On Wed, Jan 26, 2022 at 10:35:42PM -0800, Eric Biggers wrote:
> The IV passed to skcipher_request_set_crypt() above needs to be part of the
> request context, not part of the stack frame of this function, in case the xctr
> implementation is asynchronous which would cause the stack frame to go out of
> scope.  The x86 implementation operates asynchronously when called in a context
> where SIMD instructions are unavailable.
> 
> Perhaps rctx->first_block can be reused, as it's already in the request context?
> 
> Make sure to test your changes with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS enabled,
> as that is able to detect this bug (at least when CONFIG_KASAN is also enabled,
> which I also highly recommend) since it tests calling the crypto algorithms in a
> context where SIMD instructions cannot be used.  Here's the bug report I got:
> 
> 	BUG: KASAN: stack-out-of-bounds in __crypto_xor+0x29e/0x480 crypto/algapi.c:1005
> 	Read of size 8 at addr ffffc900006775f8 by task kworker/2:1/41
> 	CPU: 2 PID: 41 Comm: kworker/2:1 Not tainted 5.17.0-rc1-00071-gb35cef9ae599 #8
> 	Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ArchLinux 1.15.0-1 04/01/2014
> 	Workqueue: cryptd cryptd_queue_worker
> 	Call Trace:
> 	 <TASK>
> 	 show_stack+0x3d/0x3f arch/x86/kernel/dumpstack.c:318
> 	 __dump_stack lib/dump_stack.c:88 [inline]
> 	 dump_stack_lvl+0x49/0x5e lib/dump_stack.c:106
> 	 print_address_description.constprop.0+0x24/0x150 mm/kasan/report.c:255
> 	 __kasan_report.cold+0x7d/0x11a mm/kasan/report.c:442
> 	 kasan_report+0x3c/0x50 mm/kasan/report.c:459
> 	 __asan_report_load8_noabort+0x14/0x20 mm/kasan/report_generic.c:309
> 	 __crypto_xor+0x29e/0x480 crypto/algapi.c:1005
> 	 crypto_xor_cpy include/crypto/algapi.h:182 [inline]
> 	 xctr_crypt+0x1f1/0x2f0 arch/x86/crypto/aesni-intel_glue.c:585
> 	 crypto_skcipher_encrypt+0xe2/0x150 crypto/skcipher.c:630
> 	 cryptd_skcipher_encrypt+0x1c2/0x320 crypto/cryptd.c:274
> 	 cryptd_queue_worker+0xe4/0x160 crypto/cryptd.c:181
> 	 process_one_work+0x822/0x14e0 kernel/workqueue.c:2307
> 	 worker_thread+0x590/0xf60 kernel/workqueue.c:2454
> 	 kthread+0x257/0x2f0 kernel/kthread.c:377
> 	 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295
> 	 </TASK>
> 	Memory state around the buggy address:
> 	 ffffc90000677480: 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00
> 	 ffffc90000677500: 00 00 00 00 00 00 00 00 00 00 f3 f3 f3 f3 f3 00
> 	>ffffc90000677580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f1
> 									^
> 	 ffffc90000677600: f1 f1 f1 00 00 00 f3 f3 f3 f3 f3 00 00 00 00 00
> 	 ffffc90000677680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 	==================================================================
> 	alg: skcipher: hctr2(aes-aesni,xctr-aes-aesni,polyval-pclmulqdqni) encryption test failed (wrong result) on test vector 2, cfg="random: use_digest nosimd src_divs=[100.0%@+3830] iv_offset=45"
> 	------------[ cut here ]------------
> 	alg: self-tests for hctr2(aes-aesni,xctr-aes-aesni,polyval-pclmulqdqni) (hctr2(aes)) failed (rc=-22)
> 	WARNING: CPU: 2 PID: 519 at crypto/testmgr.c:5690 alg_test+0x2d9/0x830 crypto/testmgr.c:5690
> 
> 
> > diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> > index a3a24aa07492..fa8f33210358 100644
> > --- a/crypto/testmgr.c
> > +++ b/crypto/testmgr.c
> > @@ -4994,6 +4994,12 @@ static const struct alg_test_desc alg_test_descs[] = {
> >  		.suite = {
> >  			.hash = __VECS(ghash_tv_template)
> >  		}
> > +	}, {
> > +		.alg = "hctr2(aes)",
> > +		.test = alg_test_skcipher,
> 
> The .generic_driver field should be filled in here to allow the comparison tests
> to run, since the default strategy of forming the generic driver name isn't
> valid here; it would result in hctr2(aes-generic), which doesn't work.
> 

Note that with the above two issues fixed, it is still hanging somewhere and
never actually finishing the tests.  Maybe an infinite loop somewhere?

- Eric