Re: [PATCH] crypto: arm/aes-neonbs-cbc - Select generic cbc and aes

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]

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 <herbert@gondor.apana.org.au>
> 
> 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/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]