[PATCH] crypto: testmgr - Move crypto_simd_disabled_for_test out

[PATCH] crypto: testmgr - Move crypto_simd_disabled_for_test out

[Herbert Xu]

As testmgr is part of cryptomgr which was designed to be unloadable
as a module, it shouldn't export any symbols for other crypto
modules to use as that would prevent it from being unloaded.  All
its functionality is meant to be accessed through notifiers.

The symbol crypto_simd_disabled_for_test was added to testmgr
which caused it to be pinned as a module if its users were also
loaded.  This patch moves it out of testmgr and into crypto/simd.c
so cryptomgr can again be unloaded and replaced on demand.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/simd.c b/crypto/simd.c
index edaa479a1ec5..2027d747b746 100644
--- a/crypto/simd.c
+++ b/crypto/simd.c
@@ -47,6 +47,11 @@ struct simd_skcipher_ctx {
 	struct cryptd_skcipher *cryptd_tfm;
 };
 
+#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+DEFINE_PER_CPU(bool, crypto_simd_disabled_for_test);
+EXPORT_PER_CPU_SYMBOL_GPL(crypto_simd_disabled_for_test);
+#endif
+
 static int simd_skcipher_setkey(struct crypto_skcipher *tfm, const u8 *key,
 				unsigned int key_len)
 {
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 5831d4bbc64f..3a5a3e5cb77b 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -55,9 +55,6 @@ MODULE_PARM_DESC(noextratests, "disable expensive crypto self-tests");
 static unsigned int fuzz_iterations = 100;
 module_param(fuzz_iterations, uint, 0644);
 MODULE_PARM_DESC(fuzz_iterations, "number of fuzz test iterations");
-
-DEFINE_PER_CPU(bool, crypto_simd_disabled_for_test);
-EXPORT_PER_CPU_SYMBOL_GPL(crypto_simd_disabled_for_test);
 #endif
 
 #ifdef CONFIG_CRYPTO_MANAGER_DISABLE_TESTS
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: testmgr - Move crypto_simd_disabled_for_test out

[Eric Biggers]

On Tue, Jan 11, 2022 at 05:26:11PM +1100, Herbert Xu wrote:
> As testmgr is part of cryptomgr which was designed to be unloadable
> as a module, it shouldn't export any symbols for other crypto
> modules to use as that would prevent it from being unloaded.  All
> its functionality is meant to be accessed through notifiers.
> 
> The symbol crypto_simd_disabled_for_test was added to testmgr
> which caused it to be pinned as a module if its users were also
> loaded.  This patch moves it out of testmgr and into crypto/simd.c
> so cryptomgr can again be unloaded and replaced on demand.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> diff --git a/crypto/simd.c b/crypto/simd.c
> index edaa479a1ec5..2027d747b746 100644
> --- a/crypto/simd.c
> +++ b/crypto/simd.c
> @@ -47,6 +47,11 @@ struct simd_skcipher_ctx {
>  	struct cryptd_skcipher *cryptd_tfm;
>  };
>  
> +#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
> +DEFINE_PER_CPU(bool, crypto_simd_disabled_for_test);
> +EXPORT_PER_CPU_SYMBOL_GPL(crypto_simd_disabled_for_test);
> +#endif
> +
>  static int simd_skcipher_setkey(struct crypto_skcipher *tfm, const u8 *key,
>  				unsigned int key_len)
>  {

It doesn't look like all the users of crypto_simd_usable() select CRYPTO_SIMD.
So this will cause a build break in some configurations.

Maybe CRYPTO_MANAGER_EXTRA_TESTS should select CRYPTO_SIMD?

- Eric

Re: [PATCH] crypto: testmgr - Move crypto_simd_disabled_for_test out

[Herbert Xu]

On Tue, Jan 11, 2022 at 01:43:58PM -0800, Eric Biggers wrote:
>
> Maybe CRYPTO_MANAGER_EXTRA_TESTS should select CRYPTO_SIMD?

You're right.  I was focusing only on the module dependencies
but neglected to change the Kconfig dependencies.

I'll fix this in the next version.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[v2 PATCH] crypto: testmgr - Move crypto_simd_disabled_for_test out

[Herbert Xu]

On Wed, Jan 12, 2022 at 11:58:00AM +1100, Herbert Xu wrote:
> On Tue, Jan 11, 2022 at 01:43:58PM -0800, Eric Biggers wrote:
> >
> > Maybe CRYPTO_MANAGER_EXTRA_TESTS should select CRYPTO_SIMD?
> 
> You're right.  I was focusing only on the module dependencies
> but neglected to change the Kconfig dependencies.
> 
> I'll fix this in the next version.

---8<---
As testmgr is part of cryptomgr which was designed to be unloadable
as a module, it shouldn't export any symbols for other crypto
modules to use as that would prevent it from being unloaded.  All
its functionality is meant to be accessed through notifiers.

The symbol crypto_simd_disabled_for_test was added to testmgr
which caused it to be pinned as a module if its users were also
loaded.  This patch moves it out of testmgr and into crypto/algapi.c
so cryptomgr can again be unloaded and replaced on demand.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/algapi.c b/crypto/algapi.c
index a366cb3e8aa1..9f15e11f5d73 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -6,6 +6,7 @@
  */
 
 #include <crypto/algapi.h>
+#include <crypto/internal/simd.h>
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/fips.h>
@@ -21,6 +22,11 @@
 
 static LIST_HEAD(crypto_template_list);
 
+#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+DEFINE_PER_CPU(bool, crypto_simd_disabled_for_test);
+EXPORT_PER_CPU_SYMBOL_GPL(crypto_simd_disabled_for_test);
+#endif
+
 static inline void crypto_check_module_sig(struct module *mod)
 {
 	if (fips_enabled && mod && !module_sig_ok(mod))
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 5831d4bbc64f..3a5a3e5cb77b 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -55,9 +55,6 @@ MODULE_PARM_DESC(noextratests, "disable expensive crypto self-tests");
 static unsigned int fuzz_iterations = 100;
 module_param(fuzz_iterations, uint, 0644);
 MODULE_PARM_DESC(fuzz_iterations, "number of fuzz test iterations");
-
-DEFINE_PER_CPU(bool, crypto_simd_disabled_for_test);
-EXPORT_PER_CPU_SYMBOL_GPL(crypto_simd_disabled_for_test);
 #endif
 
 #ifdef CONFIG_CRYPTO_MANAGER_DISABLE_TESTS
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [v2 PATCH] crypto: testmgr - Move crypto_simd_disabled_for_test out

[Eric Biggers]

On Fri, Jan 14, 2022 at 05:40:30PM +1100, Herbert Xu wrote:
> On Wed, Jan 12, 2022 at 11:58:00AM +1100, Herbert Xu wrote:
> > On Tue, Jan 11, 2022 at 01:43:58PM -0800, Eric Biggers wrote:
> > >
> > > Maybe CRYPTO_MANAGER_EXTRA_TESTS should select CRYPTO_SIMD?
> > 
> > You're right.  I was focusing only on the module dependencies
> > but neglected to change the Kconfig dependencies.
> > 
> > I'll fix this in the next version.
> 
> ---8<---
> As testmgr is part of cryptomgr which was designed to be unloadable
> as a module, it shouldn't export any symbols for other crypto
> modules to use as that would prevent it from being unloaded.  All
> its functionality is meant to be accessed through notifiers.
> 
> The symbol crypto_simd_disabled_for_test was added to testmgr
> which caused it to be pinned as a module if its users were also
> loaded.  This patch moves it out of testmgr and into crypto/algapi.c
> so cryptomgr can again be unloaded and replaced on demand.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 

Reviewed-by: Eric Biggers <ebiggers@google.com>

- Eric