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
next prev parent 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).