[PATCH RFC 015/104] crypto: pass struct crypto_alg directly to alg_test()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



algboss/cryptomgr_schedule_test/cryptomgr_test currently pass the
algorithm to test by name -- stringly typed.

This is unfortunate as it opens all kinds of potential issues with race
conditions and ultimately the question of whether we actually tested
what we wanted to test.

I see no reason why we should pass the algorithm by name except for the
fact that the crypto API itself is stringly typed when
requesting/instantiating algorithms. Maybe there should be a way to
instantiate an algorithm by alg pointer too?

In any case, let's pass the alg directly so we know precisely what
algorithm we are actually testing.

Signed-off-by: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
---
 crypto/algboss.c  | 32 ++++++++++----------------------
 crypto/internal.h |  2 +-
 crypto/tcrypt.c   | 18 +++++++++++++++---
 crypto/testmgr.c  | 24 ++++++++++++------------
 4 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/crypto/algboss.c b/crypto/algboss.c
index 846f586889ee..31df14e37a3e 100644
--- a/crypto/algboss.c
+++ b/crypto/algboss.c
@@ -41,12 +41,6 @@ struct cryptomgr_param {
 	u32 omask;
 };
 
-struct crypto_test_param {
-	char driver[CRYPTO_MAX_ALG_NAME];
-	char alg[CRYPTO_MAX_ALG_NAME];
-	u32 type;
-};
-
 static int cryptomgr_probe(void *data)
 {
 	struct cryptomgr_param *param = data;
@@ -172,22 +166,21 @@ static int cryptomgr_schedule_probe(struct crypto_larval *larval)
 
 static int cryptomgr_test(void *data)
 {
-	struct crypto_test_param *param = data;
-	u32 type = param->type;
+	struct crypto_alg *alg = data;
 	int err;
 
-	err = alg_test(param->driver, param->alg, type, CRYPTO_ALG_TESTED);
+	err = alg_test(alg, alg->cra_driver_name, alg->cra_name,
+		alg->cra_flags, CRYPTO_ALG_TESTED);
 
-	crypto_alg_tested(param->driver, err);
+	crypto_alg_tested(alg->cra_driver_name, err);
 
-	kfree(param);
+	crypto_mod_put(alg);
 	module_put_and_kthread_exit(0);
 }
 
 static int cryptomgr_schedule_test(struct crypto_alg *alg)
 {
 	struct task_struct *thread;
-	struct crypto_test_param *param;
 
 	if (!IS_ENABLED(CONFIG_CRYPTO_SELFTESTS))
 		return NOTIFY_DONE;
@@ -195,22 +188,17 @@ static int cryptomgr_schedule_test(struct crypto_alg *alg)
 	if (!try_module_get(THIS_MODULE))
 		goto err;
 
-	param = kzalloc(sizeof(*param), GFP_KERNEL);
-	if (!param)
+	if (!crypto_mod_get(alg))
 		goto err_put_module;
 
-	memcpy(param->driver, alg->cra_driver_name, sizeof(param->driver));
-	memcpy(param->alg, alg->cra_name, sizeof(param->alg));
-	param->type = alg->cra_flags;
-
-	thread = kthread_run(cryptomgr_test, param, "cryptomgr_test");
+	thread = kthread_run(cryptomgr_test, alg, "cryptomgr_test");
 	if (IS_ERR(thread))
-		goto err_free_param;
+		goto err_put_alg;
 
 	return NOTIFY_STOP;
 
-err_free_param:
-	kfree(param);
+err_put_alg:
+	crypto_mod_put(alg);
 err_put_module:
 	module_put(THIS_MODULE);
 err:
diff --git a/crypto/internal.h b/crypto/internal.h
index b9afd68767c1..702934c719ef 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -65,7 +65,7 @@ extern struct list_head crypto_alg_list;
 extern struct rw_semaphore crypto_alg_sem;
 extern struct blocking_notifier_head crypto_chain;
 
-int alg_test(const char *driver, const char *alg, u32 type, u32 mask);
+int alg_test(struct crypto_alg *alg, const char *driver, const char *name, u32 type, u32 mask);
 
 #if !IS_BUILTIN(CONFIG_CRYPTO_ALGAPI) || !IS_ENABLED(CONFIG_CRYPTO_SELFTESTS)
 static inline bool crypto_boot_test_finished(void)
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index d1d88debbd71..b69560f2fdef 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1436,16 +1436,28 @@ static void test_cipher_speed(const char *algo, int enc, unsigned int secs,
 				   false);
 }
 
-static inline int tcrypt_test(const char *alg)
+static inline int tcrypt_test(const char *name)
 {
 	int ret;
+	struct crypto_alg *alg;
 
-	pr_debug("testing %s\n", alg);
+	pr_debug("testing %s\n", name);
 
-	ret = alg_test(alg, alg, 0, 0);
+	alg = crypto_alg_mod_lookup(name, 0, 0);
+	if (IS_ERR(alg)) {
+		/* non-fip algs return -EAGAIN or -ENOENT in fips mode */
+		if (fips_enabled && (PTR_ERR(alg) == -EAGAIN || PTR_ERR(alg) == -ENOENT))
+			return 0;
+
+		return PTR_ERR(alg);
+	}
+
+	ret = alg_test(alg, name, name, 0, 0);
 	/* non-fips algs return -EINVAL or -ECANCELED in fips mode */
 	if (fips_enabled && (ret == -EINVAL || ret == -ECANCELED))
 		ret = 0;
+
+	crypto_mod_put(alg);
 	return ret;
 }
 
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index ab7c6724d36f..25aadf5b6690 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -61,7 +61,7 @@ MODULE_PARM_DESC(fuzz_iterations, "number of fuzz test iterations");
 #ifndef CONFIG_CRYPTO_SELFTESTS
 
 /* a perfect nop */
-int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
+int alg_test(struct crypto_alg *alg, const char *driver, const char *name, u32 type, u32 mask)
 {
 	return 0;
 }
@@ -5782,7 +5782,7 @@ static int alg_test_fips_disabled(const struct alg_test_desc *desc)
 	return !(desc->fips_allowed & FIPS_ALLOWED);
 }
 
-int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
+int alg_test(struct crypto_alg *alg, const char *driver, const char *name, u32 type, u32 mask)
 {
 	int i;
 	int j;
@@ -5798,7 +5798,7 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 	if ((type & CRYPTO_ALG_TYPE_MASK) == CRYPTO_ALG_TYPE_CIPHER) {
 		char nalg[CRYPTO_MAX_ALG_NAME];
 
-		if (snprintf(nalg, sizeof(nalg), "ecb(%s)", alg) >=
+		if (snprintf(nalg, sizeof(nalg), "ecb(%s)", name) >=
 		    sizeof(nalg))
 			return -ENAMETOOLONG;
 
@@ -5813,7 +5813,7 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 		goto test_done;
 	}
 
-	i = alg_find_test(alg);
+	i = alg_find_test(name);
 	j = alg_find_test(driver);
 	if (i < 0 && j < 0)
 		goto notest;
@@ -5838,17 +5838,17 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 		if (fips_enabled) {
 			fips_fail_notify();
 			panic("alg: self-tests for %s (driver %s) failed in fips mode!\n",
-			      alg, driver);
+			      name, driver);
 		}
 		pr_warn("alg: self-tests for %s (driver %s) failed (rc=%d)",
-			alg, driver, rc);
+			name, driver, rc);
 		WARN(rc != -ENOENT,
 		     "alg: self-tests for %s (driver %s) failed (rc=%d)",
-		     alg, driver, rc);
+		     name, driver, rc);
 	} else {
 		if (fips_enabled)
 			pr_info("alg: self-tests for %s (driver %s) passed\n",
-				alg, driver);
+				name, driver);
 	}
 
 	return rc;
@@ -5857,7 +5857,7 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 	if ((type & CRYPTO_ALG_TYPE_MASK) == CRYPTO_ALG_TYPE_LSKCIPHER) {
 		char nalg[CRYPTO_MAX_ALG_NAME];
 
-		if (snprintf(nalg, sizeof(nalg), "ecb(%s)", alg) >=
+		if (snprintf(nalg, sizeof(nalg), "ecb(%s)", name) >=
 		    sizeof(nalg))
 			goto notest2;
 
@@ -5873,14 +5873,14 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 	}
 
 notest2:
-	printk(KERN_INFO "alg: No test for %s (driver %s)\n", alg, driver);
+	printk(KERN_INFO "alg: No test for %s (driver %s)\n", name, driver);
 
 	if (type & CRYPTO_ALG_FIPS_INTERNAL)
-		return alg_fips_disabled(driver, alg);
+		return alg_fips_disabled(driver, name);
 
 	return 0;
 non_fips_alg:
-	return alg_fips_disabled(driver, alg);
+	return alg_fips_disabled(driver, name);
 }
 
 #endif /* CONFIG_CRYPTO_SELFTESTS */
-- 
2.39.3





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux