Re: [v5 PATCH 3/7] crypto: stm32 - Simplify finup

From: Linus Walleij
Date: Sun Mar 05 2023 - 17:08:42 EST

Next message: Dmitry Osipenko: "[PATCH v12 00/11] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers"
Previous message: Vegard Nossum: "[PATCH v3 7/7] Documentation/security-bugs: document document design"
In reply to: Herbert Xu: "[v5 PATCH 3/7] crypto: stm32 - Simplify finup"
Next in thread: Herbert Xu: "Re: [v5 PATCH 3/7] crypto: stm32 - Simplify finup"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Sat, Mar 4, 2023 at 10:37 AM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:

> The current finup code is unnecessarily convoluted. There is no
> need to call update and final separately as update already does
> all the necessary work on its own.
>
> Simplify this by utilising the HASH_FLAGS_FINUP bit in rctx to
> indicate only finup and use the HASH_FLAGS_FINAL bit instead to
> signify processing common to both final and finup.
>
> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

All tests sadly fail after this patch:

[ 4.699857] stm32-hash a03c2000.hash: allocated hmac(sha256) fallback
[ 4.708231] alg: ahash: stm32-hmac-sha256 test failed (wrong
result) on test vector 0, cfg="init+finup aligned buffer"
[ 4.719048] alg: self-tests for hmac(sha256) using
stm32-hmac-sha256 failed (rc=-22)
[ 4.719070] ------------[ cut here ]------------
[ 4.731602] WARNING: CPU: 0 PID: 88 at crypto/testmgr.c:5858
alg_test.part.0+0x4d0/0x4dc
[ 4.739832] alg: self-tests for hmac(sha256) using
stm32-hmac-sha256 failed (rc=-22)
[ 4.739853] Modules linked in:
[ 4.750718] CPU: 0 PID: 88 Comm: cryptomgr_test Not tainted
6.2.0-13567-g410ada7489b7 #76
[ 4.758915] Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support)
[ 4.765897] unwind_backtrace from show_stack+0x10/0x14
[ 4.771169] show_stack from dump_stack_lvl+0x40/0x4c
[ 4.776270] dump_stack_lvl from __warn+0x94/0xc0
[ 4.781022] __warn from warn_slowpath_fmt+0x118/0x164
[ 4.786199] warn_slowpath_fmt from alg_test.part.0+0x4d0/0x4dc
[ 4.792167] alg_test.part.0 from cryptomgr_test+0x18/0x38
[ 4.797696] cryptomgr_test from kthread+0xc0/0xc4
[ 4.802527] kthread from ret_from_fork+0x14/0x2c
[ 4.807257] Exception stack(0xf10b9fb0 to 0xf10b9ff8)
[ 4.812320] 9fa0: 00000000
00000000 00000000 00000000
[ 4.820510] 9fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 4.828698] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 4.835475] ---[ end trace 0000000000000000 ]---
[ 4.840713] stm32-hash a03c2000.hash: allocated sha256 fallback
[ 4.854071] alg: ahash: stm32-sha256 test failed (wrong result) on
test vector 1, cfg="init+finup aligned buffer"
[ 4.864487] alg: self-tests for sha256 using stm32-sha256 failed (rc=-22)
[ 4.864500] ------------[ cut here ]------------
[ 4.875924] WARNING: CPU: 0 PID: 98 at crypto/testmgr.c:5858
alg_test.part.0+0x4d0/0x4dc
[ 4.884108] alg: self-tests for sha256 using stm32-sha256 failed (rc=-22)
[ 4.884119] Modules linked in:
[ 4.894022] CPU: 0 PID: 98 Comm: cryptomgr_test Tainted: G W
6.2.0-13567-g410ada7489b7 #76
[ 4.903679] Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support)
[ 4.910642] unwind_backtrace from show_stack+0x10/0x14
[ 4.915884] show_stack from dump_stack_lvl+0x40/0x4c
[ 4.920954] dump_stack_lvl from __warn+0x94/0xc0
[ 4.925675] __warn from warn_slowpath_fmt+0x118/0x164
[ 4.930826] warn_slowpath_fmt from alg_test.part.0+0x4d0/0x4dc
[ 4.936764] alg_test.part.0 from cryptomgr_test+0x18/0x38
[ 4.942265] cryptomgr_test from kthread+0xc0/0xc4
[ 4.947070] kthread from ret_from_fork+0x14/0x2c
[ 4.951780] Exception stack(0xf10e1fb0 to 0xf10e1ff8)
[ 4.956830] 1fa0: 00000000
00000000 00000000 00000000
[ 4.965005] 1fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 4.973179] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 4.979850] ---[ end trace 0000000000000000 ]---
[ 5.449257] stm32-hash a03c2000.hash: allocated hmac(sha1) fallback
[ 5.458347] alg: ahash: stm32-hmac-sha1 test failed (wrong result)
on test vector 0, cfg="init+finup aligned buffer"
[ 5.469070] alg: self-tests for hmac(sha1) using stm32-hmac-sha1
failed (rc=-22)
[ 5.469112] ------------[ cut here ]------------
[ 5.481280] WARNING: CPU: 1 PID: 101 at crypto/testmgr.c:5858
alg_test.part.0+0x4d0/0x4dc
[ 5.489663] alg: self-tests for hmac(sha1) using stm32-hmac-sha1
failed (rc=-22)
[ 5.489699] Modules linked in:
[ 5.500277] CPU: 1 PID: 101 Comm: cryptomgr_test Tainted: G
W 6.2.0-13567-g410ada7489b7 #76
[ 5.510038] Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support)
[ 5.517014] unwind_backtrace from show_stack+0x10/0x14
[ 5.522287] show_stack from dump_stack_lvl+0x40/0x4c
[ 5.527390] dump_stack_lvl from __warn+0x94/0xc0
[ 5.532140] __warn from warn_slowpath_fmt+0x118/0x164
[ 5.537318] warn_slowpath_fmt from alg_test.part.0+0x4d0/0x4dc
[ 5.543284] alg_test.part.0 from cryptomgr_test+0x18/0x38
[ 5.548815] cryptomgr_test from kthread+0xc0/0xc4
[ 5.553648] kthread from ret_from_fork+0x14/0x2c
[ 5.558378] Exception stack(0xf10edfb0 to 0xf10edff8)
[ 5.563443] dfa0: 00000000
00000000 00000000 00000000
[ 5.571633] dfc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 5.579822] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 5.586511] ---[ end trace 0000000000000000 ]---
[ 5.591474] stm32-hash a03c2000.hash: allocated sha1 fallback
[ 5.610829] alg: ahash: stm32-sha1 test failed (wrong result) on
test vector 1, cfg="init+finup aligned buffer"
[ 5.621005] alg: self-tests for sha1 using stm32-sha1 failed (rc=-22)
[ 5.621014] ------------[ cut here ]------------
[ 5.632136] WARNING: CPU: 0 PID: 113 at crypto/testmgr.c:5858
alg_test.part.0+0x4d0/0x4dc
[ 5.640368] alg: self-tests for sha1 using stm32-sha1 failed (rc=-22)
[ 5.640377] Modules linked in:
[ 5.649882] CPU: 0 PID: 113 Comm: cryptomgr_test Tainted: G
W 6.2.0-13567-g410ada7489b7 #76
[ 5.659620] Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support)
[ 5.666579] unwind_backtrace from show_stack+0x10/0x14
[ 5.671817] show_stack from dump_stack_lvl+0x40/0x4c
[ 5.676881] dump_stack_lvl from __warn+0x94/0xc0
[ 5.681594] __warn from warn_slowpath_fmt+0x118/0x164
[ 5.686739] warn_slowpath_fmt from alg_test.part.0+0x4d0/0x4dc
[ 5.692669] alg_test.part.0 from cryptomgr_test+0x18/0x38
[ 5.698163] cryptomgr_test from kthread+0xc0/0xc4
[ 5.702962] kthread from ret_from_fork+0x14/0x2c
[ 5.707668] Exception stack(0xf10edfb0 to 0xf10edff8)
[ 5.712716] dfa0: 00000000
00000000 00000000 00000000
[ 5.720887] dfc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 5.729057] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 5.735717] ---[ end trace 0000000000000000 ]---

As usual the fallback works. I tried to look into it but I don't
understand this part:

@ -768,17 +773,14 @@ static int stm32_hash_final_req(struct
stm32_hash_dev *hdev)

> + if (rctx->flags & HASH_FLAGS_FINUP)
> + return stm32_hash_update_req(hdev);

stm32_hash_update_req() will do

if (!(rctx->flags & HASH_FLAGS_CPU))
return stm32_hash_dma_send(hdev);
(...)
return stm32_hash_update_cpu(hdev);

So it replaces:

> - if (!(rctx->flags & HASH_FLAGS_CPU))
> - err = stm32_hash_dma_send(hdev);
> - else
> - err = stm32_hash_xmit_cpu(hdev, rctx->buffer, buflen, 1);

But now we are calling stm32_hash_update_cpu() instead of
stm32_hash_xmit_cpu(), because that is what stm32_hash_update_cpu()
does.

Yours,
Linus Walleij

Next message: Dmitry Osipenko: "[PATCH v12 00/11] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers"
Previous message: Vegard Nossum: "[PATCH v3 7/7] Documentation/security-bugs: document document design"
In reply to: Herbert Xu: "[v5 PATCH 3/7] crypto: stm32 - Simplify finup"
Next in thread: Herbert Xu: "Re: [v5 PATCH 3/7] crypto: stm32 - Simplify finup"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]