Re: [v5 PATCH 7/7] crypto: stm32 - Save and restore between each request
From: Linus Walleij
Date: Wed Mar 08 2023 - 04:06:16 EST
On Wed, Mar 8, 2023 at 4:40 AM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, Mar 08, 2023 at 11:23:24AM +0800, Herbert Xu wrote:
> >
> > Nevermind, I know why it doesn't work. It's specific to ux500
> > where you're polling the DCAL bit. But the DCAL bit only works
> > for the final hash, it doesn't work for the intermediate state.
> >
> > Let me check how the old ux500 handled this case.
>
> Hmm, it seems to use the same bit. I guess the meaning must be
> different with ux500.
Not sure if it's that or if we were just lucky, or the tests were not
as extensive back on the old driver :/
> Could you check for me which wait_busy() call is actually failing?
> Is it the one I added right before we save the state, or is it
> something else?
It times out - always - before writing the HMAC key in
stm32_hash_xmit_cpu().
So this:
stm32_hash_set_nblw(hdev, length);
reg = stm32_hash_read(hdev, HASH_STR);
reg |= HASH_STR_DCAL;
stm32_hash_write(hdev, HASH_STR, reg);
if (hdev->flags & HASH_FLAGS_HMAC) {
- if (stm32_hash_wait_busy(hdev))
+ if (stm32_hash_wait_busy(hdev)) {
+ dev_err(hdev->dev,
+ "timeout before writing key in
stm32_hash_xmit_cpu()\n");
return -ETIMEDOUT;
+ }
stm32_hash_write_key(hdev);
}
return -EINPROGRESS;
Gives:
[ 4.812106] stm32-hash a03c2000.hash: allocated hmac(sha256) fallback
[ 5.008829] stm32-hash a03c2000.hash: timeout before writing key in
stm32_hash_xmit_cpu()
[ 5.017167] alg: ahash: stm32-hmac-sha256 final() failed with err
-110 on test vector "random: psize=0 ksize=70", cfg="random: may_sleep
use_final src_divs=[<fl"
[ 5.041034] alg: self-tests for hmac(sha256) using
stm32-hmac-sha256 failed (rc=-110)
I put error messages at all other timeouts too but they do not
trigger.
This is why non-HMAC works fine all of the time.
> If it's something perhaps we aren't restoring the state in the
> right way, because the stm32 state restoring code is quite different
> compared to the ux500 code.
It's just the HMAC that is failing so I'm puzzled, because until
this point it is essentially a clean SHA sum.
I also noticed that it only happens with long keys (more than
64 bytes) that need to set bit 16 (HASH_CR_LKEY) in the control
register.
> Could you also confirm that the old ux500 driver actually passes
> all the extra tests on your hardware? It literally saves and
> restores the state every 256 bytes :)
I had this old patch set where I tried to clean up the old driver:
https://lore.kernel.org/linux-crypto/20220816140049.102306-1-linus.walleij@xxxxxxxxxx/
I put this old patch set on top of v6.0.
First I enabled the old crypto driver without extended tests: all works fine.
Then I enabled the extended tests. It does seem like it has
the same problem!
[ 25.486878] CPU: 1 PID: 91 Comm: cryptomgr_test Not tainted
6.0.0-00016-g23791565aac5 #3
[ 25.494967] Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support)
[ 25.501932] unwind_backtrace from show_stack+0x10/0x14
[ 25.507175] show_stack from dump_stack_lvl+0x40/0x4c
[ 25.512236] dump_stack_lvl from nmi_cpu_backtrace+0xd4/0x104
[ 25.517988] nmi_cpu_backtrace from nmi_trigger_cpumask_backtrace+0xdc/0x128
[ 25.525034] nmi_trigger_cpumask_backtrace from
trigger_single_cpu_backtrace+0x24/0x2c
[ 25.532952] trigger_single_cpu_backtrace from
rcu_dump_cpu_stacks+0x10c/0x150
[ 25.540173] rcu_dump_cpu_stacks from print_cpu_stall+0x14c/0x1d0
[ 25.546268] print_cpu_stall from check_cpu_stall+0x1cc/0x26c
[ 25.552013] check_cpu_stall from rcu_sched_clock_irq+0x74/0x16c
[ 25.558017] rcu_sched_clock_irq from update_process_times+0x68/0x94
[ 25.564377] update_process_times from tick_sched_timer+0x60/0xec
[ 25.570470] tick_sched_timer from __hrtimer_run_queues+0x15c/0x218
[ 25.576737] __hrtimer_run_queues from hrtimer_interrupt+0x124/0x2b0
[ 25.583090] hrtimer_interrupt from twd_handler+0x34/0x3c
[ 25.588489] twd_handler from handle_percpu_devid_irq+0x78/0x134
[ 25.594498] handle_percpu_devid_irq from generic_handle_domain_irq+0x24/0x34
[ 25.601640] generic_handle_domain_irq from gic_handle_irq+0x74/0x88
[ 25.608000] gic_handle_irq from generic_handle_arch_irq+0x34/0x44
[ 25.614187] generic_handle_arch_irq from call_with_stack+0x18/0x20
[ 25.620462] call_with_stack from __irq_svc+0x98/0xb0
[ 25.625514] Exception stack(0xf10ad970 to 0xf10ad9b8)
[ 25.630563] d960: c1d59a00
40000013 c056bbfc 00005f32
[ 25.638734] d980: 00000000 00000008 f10ad9e0 00000020 c1f69bc0
c1f69bc0 c2364140 f10adb34
[ 25.646904] d9a0: 00000000 f10ad9c0 c056da50 c099d1ec 20000013 ffffffff
[ 25.653512] __irq_svc from _raw_spin_unlock_irqrestore+0x1c/0x20
[ 25.659605] _raw_spin_unlock_irqrestore from regmap_read+0x50/0x60
[ 25.665882] regmap_read from hash_hw_write_key+0xd8/0x100
[ 25.671377] hash_hw_write_key from init_hash_hw+0xd8/0xfc
[ 25.676862] init_hash_hw from hash_hw_final+0x88/0x36c
[ 25.682089] hash_hw_final from ahash_final+0x58/0x9c
[ 25.687138] ahash_final from do_ahash_op+0x20/0x98
[ 25.692019] do_ahash_op from test_ahash_vec_cfg+0x68c/0x984
[ 25.697677] test_ahash_vec_cfg from test_hash_vec+0x64/0x168
[ 25.703421] test_hash_vec from __alg_test_hash+0x158/0x2d8
[ 25.708991] __alg_test_hash from alg_test_hash+0xc0/0x170
[ 25.714474] alg_test_hash from alg_test.part.0+0x378/0x4b8
[ 25.720044] alg_test.part.0 from cryptomgr_test+0x24/0x44
[ 25.725537] cryptomgr_test from kthread+0xc0/0xc4
[ 25.730334] kthread from ret_from_fork+0x14/0x2c
[ 25.735036] Exception stack(0xf10adfb0 to 0xf10adff8)
[ 25.740081] dfa0: 00000000
00000000 00000000 00000000
[ 25.748252] dfc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 25.756422] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
I can't see 100% if it is the same issue but it does hang on something
related to writing keys.
So for Ux500 at least I suppose it would be best to inhibit .import and
.export when using HMAC with long keys unless I can figure out exactly
what the issue is here. I wonder if that is possible?
Or do I have to remove it from the HMAC algos altogether?
Yours,
Linus Walleij