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 <[email protected]> --- 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
