On Wed, Mar 16, 2022 at 05:37:19PM +0100, Uwe Kleine-K?nig wrote:
>
> # CONFIG_CRYPTO_CBC is not set
This was the issue. The failure occurs on registering __cbc_aes
and the reason is that the neonbs cbc-aes requirs a fallback which
isn't available due to CBC being disabled.
I have no idea why this started occurring only with the testmgr
change though as this should have been fatal all along.
---8<---
The algorithm __cbc-aes-neonbs requires a fallback so we need
to select the config options for them or otherwise it will fail
to register on boot-up.
Fixes: 00b99ad2bac2 ("crypto: arm/aes-neonbs - Use generic cbc...")
Signed-off-by: Herbert Xu <[email protected]>
diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index 2b575792363e..e4dba5461cb3 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -102,6 +102,8 @@ config CRYPTO_AES_ARM_BS
depends on KERNEL_MODE_NEON
select CRYPTO_SKCIPHER
select CRYPTO_LIB_AES
+ select CRYPTO_AES
+ select CRYPTO_CBC
select CRYPTO_SIMD
help
Use a faster and more secure NEON based implementation of AES in CBC,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Hello Herbert,
On Thu, Mar 17, 2022 at 10:55:13AM +1200, Herbert Xu wrote:
> On Wed, Mar 16, 2022 at 05:37:19PM +0100, Uwe Kleine-K?nig wrote:
> >
> > # CONFIG_CRYPTO_CBC is not set
>
> This was the issue. The failure occurs on registering __cbc_aes
> and the reason is that the neonbs cbc-aes requirs a fallback which
> isn't available due to CBC being disabled.
>
> I have no idea why this started occurring only with the testmgr
> change though as this should have been fatal all along.
Yesterday I wondered about that, too. I didn't check, but I guess
00b99ad2bac2 was introduced between the testmgr regression and its fix
and the failure looks similar enough for me to not having noticed the
difference? Or my config somehow changed somewhere there (though I used
the same defconfig in each bisection step).
> ---8<---
> The algorithm __cbc-aes-neonbs requires a fallback so we need
> to select the config options for them or otherwise it will fail
> to register on boot-up.
>
> Fixes: 00b99ad2bac2 ("crypto: arm/aes-neonbs - Use generic cbc...")
> Signed-off-by: Herbert Xu <[email protected]>
>
> diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
> index 2b575792363e..e4dba5461cb3 100644
> --- a/arch/arm/crypto/Kconfig
> +++ b/arch/arm/crypto/Kconfig
> @@ -102,6 +102,8 @@ config CRYPTO_AES_ARM_BS
> depends on KERNEL_MODE_NEON
> select CRYPTO_SKCIPHER
> select CRYPTO_LIB_AES
> + select CRYPTO_AES
> + select CRYPTO_CBC
> select CRYPTO_SIMD
> help
> Use a faster and more secure NEON based implementation of AES in CBC,
I tested your change and that indeed fixes booting for me. Thanks!
However I think there are two problems involved here, and you only fix
one of them with your patch. Your change makes
simd_skcipher_create_compat() succeed, but aes_init() still has a broken
error handling. So if I do
diff --git a/arch/arm/crypto/aes-neonbs-glue.c b/arch/arm/crypto/aes-neonbs-glue.c
index 5c6cd3c63cbc..ee9812ee33b7 100644
--- a/arch/arm/crypto/aes-neonbs-glue.c
+++ b/arch/arm/crypto/aes-neonbs-glue.c
@@ -546,6 +546,11 @@ static int __init aes_init(void)
algname = aes_algs[i].base.cra_name + 2;
drvname = aes_algs[i].base.cra_driver_name + 2;
basename = aes_algs[i].base.cra_driver_name;
+ if (i == 1) {
+ /* simulate a problem to test the error path */
+ err = -ENOENT;
+ goto unregister_simds;
+ }
simd = simd_skcipher_create_compat(algname, drvname, basename);
err = PTR_ERR(simd);
if (IS_ERR(simd))
the BUG is back.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Do, 2022-03-17 at 10:55 +1200, Herbert Xu wrote:
> On Wed, Mar 16, 2022 at 05:37:19PM +0100, Uwe Kleine-König wrote:
> >
> > # CONFIG_CRYPTO_CBC is not set
>
> This was the issue. The failure occurs on registering __cbc_aes
> and the reason is that the neonbs cbc-aes requirs a fallback which
> isn't available due to CBC being disabled.
>
> I have no idea why this started occurring only with the testmgr
> change though as this should have been fatal all along.
I think this always failed and nobody that actually had CRYPTO_AES or
CRYPTO_CBC disabled noticed that aes-neonbs-cbc did not register.
What commit adad556efcdd caused was allowing the error path in
late_initcall(aes_init) to be hit before
late_initcall(crypto_algapi_init) would start the tests.
regards
Philipp
On Thu, Mar 17, 2022 at 10:16:16AM +0100, Philipp Zabel wrote:
>
> What commit adad556efcdd caused was allowing the error path in
> late_initcall(aes_init) to be hit before
> late_initcall(crypto_algapi_init) would start the tests.
OK I know what's going on now. Yes the registration had always
failed but it was silent so nobody noticed.
What adad556efcdd did different was to create larvals pointing
to the algorithms which will hang around until all tests complete
and that is what triggers the crash during unregister (that bug
during unregister has always existed too, it's just that it
was pretty much impossible to trigger as usually there aren't
any third parties allocating tfms during the init call).
I'll continue to work on the unregister crash.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt