linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Nathan Huckleberry <nhuck@google.com>
Cc: linux-crypto@vger.kernel.org,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	linux-arm-kernel@lists.infradead.org,
	Paul Crowley <paulcrowley@google.com>,
	Sami Tolvanen <samitolvanen@google.com>
Subject: Re: [RFC PATCH 3/7] crypto: hctr2 - Add HCTR2 support
Date: Tue, 1 Feb 2022 10:25:17 -0800	[thread overview]
Message-ID: <Yfl7DQ49AUQir0QF@sol.localdomain> (raw)
In-Reply-To: <YfI9PrAGYc0v9fGg@sol.localdomain>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-02-01 18:26 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-25  1:44 [RFC PATCH 0/7] crypto: HCTR2 support Nathan Huckleberry
2022-01-25  1:44 ` [RFC PATCH 1/7] crypto: xctr - Add XCTR support Nathan Huckleberry
2022-01-27  5:28   ` Eric Biggers
2022-01-27  9:42   ` Ard Biesheuvel
2022-01-27 19:26     ` Eric Biggers
2022-01-27 19:43       ` Ard Biesheuvel
2022-01-25  1:44 ` [RFC PATCH 2/7] crypto: polyval - Add POLYVAL support Nathan Huckleberry
2022-01-27  5:19   ` Eric Biggers
2022-01-25  1:44 ` [RFC PATCH 3/7] crypto: hctr2 - Add HCTR2 support Nathan Huckleberry
2022-01-27  5:08   ` Eric Biggers
2022-01-27  5:20     ` Herbert Xu
2022-01-27  5:36       ` Eric Biggers
2022-01-27  5:40         ` Herbert Xu
2022-01-27  5:44           ` Herbert Xu
2022-01-27  6:41             ` Eric Biggers
2022-01-27  6:35   ` Eric Biggers
2022-02-01 18:25     ` Eric Biggers [this message]
2022-01-27  9:29   ` Ard Biesheuvel
2022-01-27 19:20     ` Eric Biggers
2022-01-25  1:44 ` [RFC PATCH 4/7] crypto: x86/aesni-xctr: Add accelerated implementation of XCTR Nathan Huckleberry
2022-01-25  1:44 ` [RFC PATCH 5/7] crypto: arm64/aes-xctr: " Nathan Huckleberry
2022-01-28 14:10   ` Ard Biesheuvel
2022-02-07 10:00     ` Ard Biesheuvel
2022-01-25  1:44 ` [RFC PATCH 6/7] crypto: x86/polyval: Add PCLMULQDQ accelerated implementation of POLYVAL Nathan Huckleberry
2022-02-01 18:18   ` Eric Biggers
2022-02-03  3:28   ` Eric Biggers
2022-01-25  1:44 ` [RFC PATCH 7/7] crypto: arm64/polyval: Add PMULL " Nathan Huckleberry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yfl7DQ49AUQir0QF@sol.localdomain \
    --to=ebiggers@kernel.org \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=nhuck@google.com \
    --cc=paulcrowley@google.com \
    --cc=samitolvanen@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).