LinuxLists.cc - [PATCH] crypto: api - Fix built-in testing dependency failures

[permalink] [raw]

Subject: Re: [PATCH] crypto: api - Fix built-in testing dependency failures

On Mon, Sep 13, 2021 at 11:16:03AM -0700, Eric Biggers wrote:
>
> Are there any specific examples that you could give?

The one that triggered this was ecdh-nist-p256-generic, which
calls into drbg. Both ecdh and drbg are at level subsys_initcall
so the order between them is random.

Beyond this, obviously we have already moved many algorithms
ot subsys_initcall precisely for this purpose and with this
patch, they can all be moved back to module_init.

> 'tested' is set before the algorithm has actually been tested, and it sounds
> like the same as CRYPTO_ALG_TESTED which already exists. Maybe it should be
> called something else, like 'test_started'?

Sure, I can rename that.

> Is there a way to continue iterating from the previous algorithm, so that this
> doesn't take quadratic time?

It's certainly possible to optimise this, but I'm not inclined
to do it unless someone can show me that it's a real issue :)

The simplest way to optimise this would be to create a separate
list for the test larvae.

> A comment explaining why the tests aren't run until late_initcall would be
> helpful. People shouldn't have to dig through commit messages to understand the
> code.

Sure.

> Also, did you check whether there is anything that relies on the crypto API
> being available before or during late_initcall? That wouldn't work with this
> new approach, right?

The patch is supposed to deal with that scenario by starting the
test on-demand should someone request for it before late_initcall.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2021-09-17 09:56:26

[permalink] [raw]

Subject: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

When complex algorithms that depend on other algorithms are built
into the kernel, the order of registration must be done such that
the underlying algorithms are ready before the ones on top are
registered. As otherwise they would fail during the self-test
which is required during registration.

In the past we have used subsystem initialisation ordering to
guarantee this. The number of such precedence levels are limited
and they may cause ripple effects in other subsystems.

This patch solves this problem by delaying all self-tests during
boot-up for built-in algorithms. They will be tested either when
something else in the kernel requests for them, or when we have
finished registering all built-in algorithms, whichever comes
earlier.

Reported-by: Vladis Dronov <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>

diff --git a/crypto/algapi.c b/crypto/algapi.c
index 43f999dba4dc..422bdca214e1 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -389,29 +389,10 @@ void crypto_remove_final(struct list_head *list)
}
EXPORT_SYMBOL_GPL(crypto_remove_final);

-static void crypto_wait_for_test(struct crypto_larval *larval)
-{
- int err;
-
- err = crypto_probing_notify(CRYPTO_MSG_ALG_REGISTER, larval->adult);
- if (err != NOTIFY_STOP) {
- if (WARN_ON(err != NOTIFY_DONE))
- goto out;
- crypto_alg_tested(larval->alg.cra_driver_name, 0);
- }
-
- err = wait_for_completion_killable(&larval->completion);
- WARN_ON(err);
- if (!err)
- crypto_notify(CRYPTO_MSG_ALG_LOADED, larval);
-
-out:
- crypto_larval_kill(&larval->alg);
-}
-
int crypto_register_alg(struct crypto_alg *alg)
{
struct crypto_larval *larval;
+ bool test_started;
int err;

alg->cra_flags &= ~CRYPTO_ALG_DEAD;
@@ -421,12 +402,15 @@ int crypto_register_alg(struct crypto_alg *alg)

down_write(&crypto_alg_sem);
larval = __crypto_register_alg(alg);
+ test_started = static_key_enabled(&crypto_boot_test_finished);
+ larval->test_started = test_started;
up_write(&crypto_alg_sem);

if (IS_ERR(larval))
return PTR_ERR(larval);

- crypto_wait_for_test(larval);
+ if (test_started)
+ crypto_wait_for_test(larval);
return 0;
}
EXPORT_SYMBOL_GPL(crypto_register_alg);
@@ -633,6 +617,8 @@ int crypto_register_instance(struct crypto_template *tmpl,
if (IS_ERR(larval))
goto unlock;

+ larval->test_started = true;
+
hlist_add_head(&inst->list, &tmpl->instances);
inst->tmpl = tmpl;

@@ -1261,9 +1247,48 @@ void crypto_stats_skcipher_decrypt(unsigned int cryptlen, int ret,
EXPORT_SYMBOL_GPL(crypto_stats_skcipher_decrypt);
#endif

+static void __init crypto_start_tests(void)
+{
+ for (;;) {
+ struct crypto_larval *larval = NULL;
+ struct crypto_alg *q;
+
+ down_write(&crypto_alg_sem);
+
+ list_for_each_entry(q, &crypto_alg_list, cra_list) {
+ struct crypto_larval *l;
+
+ if (!crypto_is_larval(q))
+ continue;
+
+ l = (void *)q;
+
+ if (!crypto_is_test_larval(l))
+ continue;
+
+ if (l->test_started)
+ continue;
+
+ l->test_started = true;
+ larval = l;
+ break;
+ }
+
+ up_write(&crypto_alg_sem);
+
+ if (!larval)
+ break;
+
+ crypto_wait_for_test(larval);
+ }
+
+ static_branch_enable(&crypto_boot_test_finished);
+}
+
static int __init crypto_algapi_init(void)
{
crypto_init_proc();
+ crypto_start_tests();
return 0;
}

@@ -1272,7 +1297,11 @@ static void __exit crypto_algapi_exit(void)
crypto_exit_proc();
}

-module_init(crypto_algapi_init);
+/*
+ * We run this at late_initcall so that all the built-in algorithms
+ * have had a chance to register themselves first.
+ */
+late_initcall(crypto_algapi_init);
module_exit(crypto_algapi_exit);

MODULE_LICENSE("GPL");
diff --git a/crypto/api.c b/crypto/api.c
index c4eda56cff89..1cf1f03347cc 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -12,6 +12,7 @@

#include <linux/err.h>
#include <linux/errno.h>
+#include <linux/jump_label.h>
#include <linux/kernel.h>
#include <linux/kmod.h>
#include <linux/module.h>
@@ -30,6 +31,8 @@ EXPORT_SYMBOL_GPL(crypto_alg_sem);
BLOCKING_NOTIFIER_HEAD(crypto_chain);
EXPORT_SYMBOL_GPL(crypto_chain);

+DEFINE_STATIC_KEY_FALSE(crypto_boot_test_finished);
+
static struct crypto_alg *crypto_larval_wait(struct crypto_alg *alg);

struct crypto_alg *crypto_mod_get(struct crypto_alg *alg)
@@ -47,11 +50,6 @@ void crypto_mod_put(struct crypto_alg *alg)
}
EXPORT_SYMBOL_GPL(crypto_mod_put);

-static inline int crypto_is_test_larval(struct crypto_larval *larval)
-{
- return larval->alg.cra_driver_name[0];
-}
-
static struct crypto_alg *__crypto_alg_lookup(const char *name, u32 type,
u32 mask)
{
@@ -163,11 +161,55 @@ void crypto_larval_kill(struct crypto_alg *alg)
}
EXPORT_SYMBOL_GPL(crypto_larval_kill);

+void crypto_wait_for_test(struct crypto_larval *larval)
+{
+ int err;
+
+ err = crypto_probing_notify(CRYPTO_MSG_ALG_REGISTER, larval->adult);
+ if (err != NOTIFY_STOP) {
+ if (WARN_ON(err != NOTIFY_DONE))
+ goto out;
+ crypto_alg_tested(larval->alg.cra_driver_name, 0);
+ }
+
+ err = wait_for_completion_killable(&larval->completion);
+ WARN_ON(err);
+ if (!err)
+ crypto_notify(CRYPTO_MSG_ALG_LOADED, larval);
+
+out:
+ crypto_larval_kill(&larval->alg);
+}
+EXPORT_SYMBOL_GPL(crypto_wait_for_test);
+
+static void crypto_start_test(struct crypto_larval *larval)
+{
+ if (!crypto_is_test_larval(larval))
+ return;
+
+ if (larval->test_started)
+ return;
+
+ down_write(&crypto_alg_sem);
+ if (larval->test_started) {
+ up_write(&crypto_alg_sem);
+ return;
+ }
+
+ larval->test_started = true;
+ up_write(&crypto_alg_sem);
+
+ crypto_wait_for_test(larval);
+}
+
static struct crypto_alg *crypto_larval_wait(struct crypto_alg *alg)
{
struct crypto_larval *larval = (void *)alg;
long timeout;

+ if (!static_branch_likely(&crypto_boot_test_finished))
+ crypto_start_test(larval);
+
timeout = wait_for_completion_killable_timeout(
&larval->completion, 60 * HZ);

diff --git a/crypto/internal.h b/crypto/internal.h
index f00869af689f..c08385571853 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -10,6 +10,7 @@

#include <crypto/algapi.h>
#include <linux/completion.h>
+#include <linux/jump_label.h>
#include <linux/list.h>
#include <linux/module.h>
#include <linux/notifier.h>
@@ -27,6 +28,7 @@ struct crypto_larval {
struct crypto_alg *adult;
struct completion completion;
u32 mask;
+ bool test_started;
};

enum {
@@ -45,6 +47,8 @@ extern struct list_head crypto_alg_list;
extern struct rw_semaphore crypto_alg_sem;
extern struct blocking_notifier_head crypto_chain;

+DECLARE_STATIC_KEY_FALSE(crypto_boot_test_finished);
+
#ifdef CONFIG_PROC_FS
void __init crypto_init_proc(void);
void __exit crypto_exit_proc(void);
@@ -70,6 +74,7 @@ struct crypto_alg *crypto_alg_mod_lookup(const char *name, u32 type, u32 mask);

struct crypto_larval *crypto_larval_alloc(const char *name, u32 type, u32 mask);
void crypto_larval_kill(struct crypto_alg *alg);
+void crypto_wait_for_test(struct crypto_larval *larval);
void crypto_alg_tested(const char *name, int err);

void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
@@ -156,5 +161,10 @@ static inline void crypto_yield(u32 flags)
cond_resched();
}

+static inline int crypto_is_test_larval(struct crypto_larval *larval)
+{
+ return larval->alg.cra_driver_name[0];
+}
+
#endif /* _CRYPTO_INTERNAL_H */

--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2021-09-28 18:33:28

by Nathan Chancellor

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Fri, Sep 17, 2021 at 08:26:19AM +0800, Herbert Xu wrote:
> When complex algorithms that depend on other algorithms are built
> into the kernel, the order of registration must be done such that
> the underlying algorithms are ready before the ones on top are
> registered. As otherwise they would fail during the self-test
> which is required during registration.
>
> In the past we have used subsystem initialisation ordering to
> guarantee this. The number of such precedence levels are limited
> and they may cause ripple effects in other subsystems.
>
> This patch solves this problem by delaying all self-tests during
> boot-up for built-in algorithms. They will be tested either when
> something else in the kernel requests for them, or when we have
> finished registering all built-in algorithms, whichever comes
> earlier.
>
> Reported-by: Vladis Dronov <[email protected]>
> Signed-off-by: Herbert Xu <[email protected]>

This patch as commit 3cefb01905df ("crypto: api - Fix built-in testing
dependency failures") in -next (along with the follow up fix) causes the
following depmod error:

$ make -skj"$(nproc)" ARCH=powerpc CROSS_COMPILE=powerpc-linux- INSTALL_MOD_PATH=rootfs ppc44x_defconfig all modules_install
depmod: ERROR: Cycle detected: crypto -> crypto_algapi -> crypto
depmod: ERROR: Found 2 modules in dependency cycles!
make: *** [Makefile:1946: modules_install] Error 1

Initially reported on our CI:

https://github.com/ClangBuiltLinux/continuous-integration2/runs/3732847295?check_suite_focus=true

Cheers,
Nathan

2021-10-01 06:03:56

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Tue, Sep 28, 2021 at 11:32:09AM -0700, Nathan Chancellor wrote:
>
> This patch as commit 3cefb01905df ("crypto: api - Fix built-in testing
> dependency failures") in -next (along with the follow up fix) causes the
> following depmod error:
>
> $ make -skj"$(nproc)" ARCH=powerpc CROSS_COMPILE=powerpc-linux- INSTALL_MOD_PATH=rootfs ppc44x_defconfig all modules_install
> depmod: ERROR: Cycle detected: crypto -> crypto_algapi -> crypto
> depmod: ERROR: Found 2 modules in dependency cycles!
> make: *** [Makefile:1946: modules_install] Error 1
>
> Initially reported on our CI:
>
> https://github.com/ClangBuiltLinux/continuous-integration2/runs/3732847295?check_suite_focus=true

That's weird, I can't reproduce this. Where can I find your Kconfig
file? Alternatively, can you identify exactly what is in algapi that
is being depended on by crypto?

The crypto module should be at the very base and there should be no
depenedencies from it on algapi. The algapi module is meant to be
on top of crypto obviously.

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

2021-10-01 11:16:35

by Naresh Kamboju

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

Hi Herbert,

I do see the reported problem while building modules.
you may use the steps to reproduce.

On Fri, 1 Oct 2021 at 11:58, Herbert Xu <[email protected]> wrote:
>
> On Tue, Sep 28, 2021 at 11:32:09AM -0700, Nathan Chancellor wrote:
> >
> > This patch as commit 3cefb01905df ("crypto: api - Fix built-in testing
> > dependency failures") in -next (along with the follow up fix) causes the
> > following depmod error:
> >
> > $ make -skj"$(nproc)" ARCH=powerpc CROSS_COMPILE=powerpc-linux- INSTALL_MOD_PATH=rootfs ppc44x_defconfig all modules_install
> > depmod: ERROR: Cycle detected: crypto -> crypto_algapi -> crypto
> > depmod: ERROR: Found 2 modules in dependency cycles!
> > make: *** [Makefile:1946: modules_install] Error 1
> >
> > Initially reported on our CI:
> >
> > https://github.com/ClangBuiltLinux/continuous-integration2/runs/3732847295?check_suite_focus=true
>
> That's weird, I can't reproduce this. Where can I find your Kconfig
> file? Alternatively, can you identify exactly what is in algapi that
> is being depended on by crypto?
>
> The crypto module should be at the very base and there should be no
> depenedencies from it on algapi. The algapi module is meant to be
> on top of crypto obviously.

#!/bin/sh

# TuxMake is a command line tool and Python library that provides
# portable and repeatable Linux kernel builds across a variety of
# architectures, toolchains, kernel configurations, and make targets.
#
# TuxMake supports the concept of runtimes.
# See https://docs.tuxmake.org/runtimes/, for that to work it requires
# that you install podman or docker on your system.
#
# To install tuxmake on your system globally:
# sudo pip3 install -U tuxmake
#
# See https://docs.tuxmake.org/ for complete documentation.

tuxmake --runtime podman --target-arch mips --toolchain gcc-10
--kconfig rt305x_defconfig

# to reproduce this build locally: tuxmake --target-arch=mips
--kconfig=rt305x_defconfig --toolchain=gcc-10 --wrapper=sccache
--environment=KBUILD_BUILD_TIMESTAMP=@1633074287
--environment=KBUILD_BUILD_USER=tuxmake
--environment=KBUILD_BUILD_HOST=tuxmake
--environment=SCCACHE_BUCKET=sccache.tuxbuild.com --runtime=podman
--image=855116176053.dkr.ecr.us-east-1.amazonaws.com/tuxmake/mips_gcc-10
config default kernel xipkernel modules dtbs dtbs-legacy debugkernel
headers
make --silent --keep-going --jobs=8
O=/home/tuxbuild/.cache/tuxmake/builds/current ARCH=mips
CROSS_COMPILE=mips-linux-gnu- 'CC=sccache mips-linux-gnu-gcc'
'HOSTCC=sccache gcc' rt305x_defconfig
make --silent --keep-going --jobs=8
O=/home/tuxbuild/.cache/tuxmake/builds/current ARCH=mips
CROSS_COMPILE=mips-linux-gnu- 'CC=sccache mips-linux-gnu-gcc'
'HOSTCC=sccache gcc'
make --silent --keep-going --jobs=8
O=/home/tuxbuild/.cache/tuxmake/builds/current ARCH=mips
CROSS_COMPILE=mips-linux-gnu- 'CC=sccache mips-linux-gnu-gcc'
'HOSTCC=sccache gcc' uImage.gz
make --silent --keep-going --jobs=8
O=/home/tuxbuild/.cache/tuxmake/builds/current ARCH=mips
CROSS_COMPILE=mips-linux-gnu- 'CC=sccache mips-linux-gnu-gcc'
'HOSTCC=sccache gcc' modules_install INSTALL_MOD_STRIP=1
INSTALL_MOD_PATH=/home/tuxbuild/.cache/tuxmake/builds/current/modinstall
depmod: ERROR: Cycle detected: crypto -> crypto_algapi -> crypto
depmod: ERROR: Found 2 modules in dependency cycles!
make[1]: *** [/builds/linux/Makefile:1961: modules_install] Error 1
make: *** [Makefile:226: __sub-make] Error 2
make: Target 'modules_install' not remade because of errors.

--
Linaro LKFT
https://lkft.linaro.org

2021-10-01 18:03:47

by Nathan Chancellor

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Fri, Oct 01, 2021 at 01:50:58PM +0800, Herbert Xu wrote:
> On Tue, Sep 28, 2021 at 11:32:09AM -0700, Nathan Chancellor wrote:
> >
> > This patch as commit 3cefb01905df ("crypto: api - Fix built-in testing
> > dependency failures") in -next (along with the follow up fix) causes the
> > following depmod error:
> >
> > $ make -skj"$(nproc)" ARCH=powerpc CROSS_COMPILE=powerpc-linux- INSTALL_MOD_PATH=rootfs ppc44x_defconfig all modules_install
> > depmod: ERROR: Cycle detected: crypto -> crypto_algapi -> crypto
> > depmod: ERROR: Found 2 modules in dependency cycles!
> > make: *** [Makefile:1946: modules_install] Error 1
> >
> > Initially reported on our CI:
> >
> > https://github.com/ClangBuiltLinux/continuous-integration2/runs/3732847295?check_suite_focus=true
>
> That's weird, I can't reproduce this. Where can I find your Kconfig
> file? Alternatively, can you identify exactly what is in algapi that
> is being depended on by crypto?

I have attached the Kconfig file that I used to reproduce it. It is
still reproducible for me at your latest commit in cryptodev
(e42dff467ee688fe6b5a083f1837d06e3b27d8c0) with that exact command that
I gave you.

It is possible that it could be crypto_boot_test_finished?

Cheers,
Nathan

Attachments:

(No filename) (1.24 kB)
.config (72.50 kB)
Download all attachments

2021-10-03 00:37:14

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Fri, Oct 01, 2021 at 11:01:59AM -0700, Nathan Chancellor wrote:
>
> I have attached the Kconfig file that I used to reproduce it. It is
> still reproducible for me at your latest commit in cryptodev
> (e42dff467ee688fe6b5a083f1837d06e3b27d8c0) with that exact command that
> I gave you.
>
> It is possible that it could be crypto_boot_test_finished?

I don't think that's the issue because algapi already depends on
api. However, the softdep on cryptomgr in api looks suspicious,
as it would always introduce a loop. Can you try removing that
softdep from api.c and see if the problem resolves it self?

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

2021-10-06 02:36:17

by Nathan Chancellor

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Sun, Oct 03, 2021 at 08:28:01AM +0800, Herbert Xu wrote:
> On Fri, Oct 01, 2021 at 11:01:59AM -0700, Nathan Chancellor wrote:
> >
> > I have attached the Kconfig file that I used to reproduce it. It is
> > still reproducible for me at your latest commit in cryptodev
> > (e42dff467ee688fe6b5a083f1837d06e3b27d8c0) with that exact command that
> > I gave you.
> >
> > It is possible that it could be crypto_boot_test_finished?
>
> I don't think that's the issue because algapi already depends on
> api. However, the softdep on cryptomgr in api looks suspicious,
> as it would always introduce a loop. Can you try removing that
> softdep from api.c and see if the problem resolves it self?

I assume this is the diff you mean? This does not resolve the issue. My
apologies if I am slow to respond, I am on vacation until the middle of
next week.

Cheers,
Nathan

diff --git a/crypto/api.c b/crypto/api.c
index ee5991fe11f8..e3e87c37f996 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -646,4 +646,3 @@ EXPORT_SYMBOL_GPL(crypto_req_done);

MODULE_DESCRIPTION("Cryptographic core API");
MODULE_LICENSE("GPL");
-MODULE_SOFTDEP("pre: cryptomgr");

2021-10-19 13:29:30

[permalink] [raw]

Subject: [PATCH] crypto: api - Do not create test larvals if manager is disabled

On Tue, Oct 05, 2021 at 07:33:28PM -0700, Nathan Chancellor wrote:
>
> I assume this is the diff you mean? This does not resolve the issue. My
> apologies if I am slow to respond, I am on vacation until the middle of
> next week.

Sorry for the delay. The kernel robot figured out the problem
for me. It's the crypto_alg_tested call that causes api.c to
depend on algapi.c. This call is only invoked in the case where
the crypto manager is turned off. We could instead simply make
test larvals disappear in that case.

---8<---
The delayed boot-time testing patch created a dependency loop
between api.c and algapi.c because it added a crypto_alg_tested
call to the former when the crypto manager is disabled.

We could instead avoid creating the test larvals if the crypto
manager is disabled. This avoids the dependency loop as well
as saving some unnecessary work, albeit in a very unlikely case.

Reported-by: Nathan Chancellor <[email protected]>
Reported-by: Naresh Kamboju <[email protected]>
Reported-by: kernel test robot <[email protected]>
Fixes: adad556efcdd ("crypto: api - Fix built-in testing dependency failures")
Signed-off-by: Herbert Xu <[email protected]>

diff --git a/crypto/algapi.c b/crypto/algapi.c
index 422bdca214e1..d379fd91fb7b 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -216,6 +216,32 @@ void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
}
EXPORT_SYMBOL_GPL(crypto_remove_spawns);

+static struct crypto_larval *crypto_alloc_test_larval(struct crypto_alg *alg)
+{
+ struct crypto_larval *larval;
+
+ if (!IS_ENABLED(CONFIG_CRYPTO_MANAGER))
+ return NULL;
+
+ larval = crypto_larval_alloc(alg->cra_name,
+ alg->cra_flags | CRYPTO_ALG_TESTED, 0);
+ if (IS_ERR(larval))
+ return larval;
+
+ larval->adult = crypto_mod_get(alg);
+ if (!larval->adult) {
+ kfree(larval);
+ return ERR_PTR(-ENOENT);
+ }
+
+ refcount_set(&larval->alg.cra_refcnt, 1);
+ memcpy(larval->alg.cra_driver_name, alg->cra_driver_name,
+ CRYPTO_MAX_ALG_NAME);
+ larval->alg.cra_priority = alg->cra_priority;
+
+ return larval;
+}
+
static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
{
struct crypto_alg *q;
@@ -250,31 +276,20 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
goto err;
}

- larval = crypto_larval_alloc(alg->cra_name,
- alg->cra_flags | CRYPTO_ALG_TESTED, 0);
+ larval = crypto_alloc_test_larval(alg);
if (IS_ERR(larval))
goto out;

- ret = -ENOENT;
- larval->adult = crypto_mod_get(alg);
- if (!larval->adult)
- goto free_larval;
-
- refcount_set(&larval->alg.cra_refcnt, 1);
- memcpy(larval->alg.cra_driver_name, alg->cra_driver_name,
- CRYPTO_MAX_ALG_NAME);
- larval->alg.cra_priority = alg->cra_priority;
-
list_add(&alg->cra_list, &crypto_alg_list);
- list_add(&larval->alg.cra_list, &crypto_alg_list);
+
+ if (larval)
+ list_add(&larval->alg.cra_list, &crypto_alg_list);

crypto_stats_init(alg);

out:
return larval;

-free_larval:
- kfree(larval);
err:
larval = ERR_PTR(ret);
goto out;
@@ -403,10 +418,11 @@ int crypto_register_alg(struct crypto_alg *alg)
down_write(&crypto_alg_sem);
larval = __crypto_register_alg(alg);
test_started = static_key_enabled(&crypto_boot_test_finished);
- larval->test_started = test_started;
+ if (!IS_ERR_OR_NULL(larval))
+ larval->test_started = test_started;
up_write(&crypto_alg_sem);

- if (IS_ERR(larval))
+ if (IS_ERR_OR_NULL(larval))
return PTR_ERR(larval);

if (test_started)
@@ -616,8 +632,8 @@ int crypto_register_instance(struct crypto_template *tmpl,
larval = __crypto_register_alg(&inst->alg);
if (IS_ERR(larval))
goto unlock;
-
- larval->test_started = true;
+ else if (larval)
+ larval->test_started = true;

hlist_add_head(&inst->list, &tmpl->instances);
inst->tmpl = tmpl;
@@ -626,7 +642,7 @@ int crypto_register_instance(struct crypto_template *tmpl,
up_write(&crypto_alg_sem);

err = PTR_ERR(larval);
- if (IS_ERR(larval))
+ if (IS_ERR_OR_NULL(larval))
goto err;

crypto_wait_for_test(larval);
diff --git a/crypto/api.c b/crypto/api.c
index ee5991fe11f8..cf0869dd130b 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -167,11 +167,8 @@ void crypto_wait_for_test(struct crypto_larval *larval)
int err;

err = crypto_probing_notify(CRYPTO_MSG_ALG_REGISTER, larval->adult);
- if (err != NOTIFY_STOP) {
- if (WARN_ON(err != NOTIFY_DONE))
- goto out;
- crypto_alg_tested(larval->alg.cra_driver_name, 0);
- }
+ if (WARN_ON_ONCE(err != NOTIFY_STOP))
+ goto out;

err = wait_for_completion_killable(&larval->completion);
WARN_ON(err);
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2021-10-26 22:10:25

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

Hi,

On Fri, Sep 17, 2021 at 08:26:19AM +0800, Herbert Xu wrote:
> When complex algorithms that depend on other algorithms are built
> into the kernel, the order of registration must be done such that
> the underlying algorithms are ready before the ones on top are
> registered. As otherwise they would fail during the self-test
> which is required during registration.
>
> In the past we have used subsystem initialisation ordering to
> guarantee this. The number of such precedence levels are limited
> and they may cause ripple effects in other subsystems.
>
> This patch solves this problem by delaying all self-tests during
> boot-up for built-in algorithms. They will be tested either when
> something else in the kernel requests for them, or when we have
> finished registering all built-in algorithms, whichever comes
> earlier.
>
> Reported-by: Vladis Dronov <[email protected]>
> Signed-off-by: Herbert Xu <[email protected]>
>

I can not explain it, but this patch causes a crash with one of my boot
tests (riscv32 with riscv32 virt machine and e1000 network adapter):

[ 9.948557] e1000 0000:00:01.0: enabling device (0000 -> 0003)
[ 9.968578] Unable to handle kernel paging request at virtual address 9e000000
[ 9.969207] Oops [#1]
[ 9.969325] Modules linked in:
[ 9.969619] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc6-next-20211025 #1
[ 9.969983] Hardware name: riscv-virtio,qemu (DT)
[ 9.970262] epc : e1000_io_write+0x10/0x1c
[ 9.970487] ra : e1000_reset_hw+0xfa/0x312
[ 9.970639] epc : c07b3a44 ra : c07b5e4a sp : c258dcf0
[ 9.970792] gp : c1d6cfa0 tp : c25b0040 t0 : c1f05b3c
[ 9.970941] t1 : 04d6d7d4 t2 : 00001fff s0 : c258dd00
[ 9.971091] s1 : c36a9990 a0 : c36a9990 a1 : 9e000000
[ 9.971240] a2 : 00000000 a3 : 04000000 a4 : 00000002
[ 9.971389] a5 : 9e000000 a6 : 00000000 a7 : 00006000
[ 9.971539] s2 : c101b3ec s3 : c23aceb0 s4 : 04140240
[ 9.971692] s5 : 00000000 s6 : c14a3550 s7 : c1d72000
[ 9.971872] s8 : 00000000 s9 : c36a9000 s10: 00000000
[ 9.972037] s11: 00000000 t3 : cb75ee6c t4 : 0000000c
[ 9.972200] t5 : 000021cb t6 : c1f017a0
[ 9.972336] status: 00000120 badaddr: 9e000000 cause: 0000000f
[ 9.972570] [<c07b3a44>] e1000_io_write+0x10/0x1c
[ 9.973382] ---[ end trace 49388ec34793549e ]---
[ 9.973873] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

Bisect log is attached. Reverting this patch fixes the problem. The problem
is always seen with this patch applied, and is never seen with this patch
reverted.

Any idea what might be going on, and how to debug the problem ?

Thanks,
Guenter

---
# bad: [2376e5fe91bcad74b997d2cc0535abff79ec73c5] Add linux-next specific files for 20211026
# good: [3906fe9bb7f1a2c8667ae54e967dc8690824f4ea] Linux 5.15-rc7
git bisect start 'HEAD' 'v5.15-rc7'
# bad: [18298270669947b661fe47bf7ec755a6d254c464] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
git bisect bad 18298270669947b661fe47bf7ec755a6d254c464
# good: [7294cee5cd18f89b0070ac8b0cd872cc663896de] Merge branch 'i3c/next' of git://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git
git bisect good 7294cee5cd18f89b0070ac8b0cd872cc663896de
# good: [a7021af707a3637c963ce41802b650db6793eb8a] usb: smsc: use eth_hw_addr_set()
git bisect good a7021af707a3637c963ce41802b650db6793eb8a
# good: [5c511d28b9596fda6c550b0f0c3b163f6dac7e54] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git
git bisect good 5c511d28b9596fda6c550b0f0c3b163f6dac7e54
# good: [0969becb5f7661fb0db1a5d6b60f3d7f046ff6a7] s390/qeth: improve trace entries for MAC address (un)registration
git bisect good 0969becb5f7661fb0db1a5d6b60f3d7f046ff6a7
# good: [57edc4d2baac9210564ffe8ea333aabacdce650c] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
git bisect good 57edc4d2baac9210564ffe8ea333aabacdce650c
# good: [a84f7cc76f5d33450e9fc6e681df1e1bf716773e] Merge branch 'nand/next' of git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
git bisect good a84f7cc76f5d33450e9fc6e681df1e1bf716773e
# bad: [38aa192a05f22f9778f9420e630f0322525ef12e] crypto: ecc - fix CRYPTO_DEFAULT_RNG dependency
git bisect bad 38aa192a05f22f9778f9420e630f0322525ef12e
# good: [ba79a32acfde1ffdaefc05b02420c4124b60dbd3] crypto: qat - replace deprecated MSI API
git bisect good ba79a32acfde1ffdaefc05b02420c4124b60dbd3
# good: [81f53028dfbc79844f727a7c13d337ba827a471c] crypto: drbg - Fix unused value warning in drbg_healthcheck_sanity()
git bisect good 81f53028dfbc79844f727a7c13d337ba827a471c
# good: [ca605f97dae4bf070b7c584aec23c1c922e4d823] crypto: qat - power up 4xxx device
git bisect good ca605f97dae4bf070b7c584aec23c1c922e4d823
# bad: [adad556efcdd42a1d9e060cbe5f6161cccf1fa28] crypto: api - Fix built-in testing dependency failures
git bisect bad adad556efcdd42a1d9e060cbe5f6161cccf1fa28
# good: [7c5329697ed4e0e1bf9a4e4fc9f0053f2f58935d] crypto: marvell/cesa - drop unneeded MODULE_ALIAS
git bisect good 7c5329697ed4e0e1bf9a4e4fc9f0053f2f58935d
# first bad commit: [adad556efcdd42a1d9e060cbe5f6161cccf1fa28] crypto: api - Fix built-in testing dependency failures

2021-10-27 14:37:20

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Tue, Oct 26, 2021 at 09:33:19AM -0700, Guenter Roeck wrote:
>
> I can not explain it, but this patch causes a crash with one of my boot
> tests (riscv32 with riscv32 virt machine and e1000 network adapter):
>
> [ 9.948557] e1000 0000:00:01.0: enabling device (0000 -> 0003)
> [ 9.968578] Unable to handle kernel paging request at virtual address 9e000000
> [ 9.969207] Oops [#1]
> [ 9.969325] Modules linked in:
> [ 9.969619] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc6-next-20211025 #1
> [ 9.969983] Hardware name: riscv-virtio,qemu (DT)
> [ 9.970262] epc : e1000_io_write+0x10/0x1c
> [ 9.970487] ra : e1000_reset_hw+0xfa/0x312
> [ 9.970639] epc : c07b3a44 ra : c07b5e4a sp : c258dcf0
> [ 9.970792] gp : c1d6cfa0 tp : c25b0040 t0 : c1f05b3c
> [ 9.970941] t1 : 04d6d7d4 t2 : 00001fff s0 : c258dd00
> [ 9.971091] s1 : c36a9990 a0 : c36a9990 a1 : 9e000000
> [ 9.971240] a2 : 00000000 a3 : 04000000 a4 : 00000002
> [ 9.971389] a5 : 9e000000 a6 : 00000000 a7 : 00006000
> [ 9.971539] s2 : c101b3ec s3 : c23aceb0 s4 : 04140240
> [ 9.971692] s5 : 00000000 s6 : c14a3550 s7 : c1d72000
> [ 9.971872] s8 : 00000000 s9 : c36a9000 s10: 00000000
> [ 9.972037] s11: 00000000 t3 : cb75ee6c t4 : 0000000c
> [ 9.972200] t5 : 000021cb t6 : c1f017a0
> [ 9.972336] status: 00000120 badaddr: 9e000000 cause: 0000000f
> [ 9.972570] [<c07b3a44>] e1000_io_write+0x10/0x1c
> [ 9.973382] ---[ end trace 49388ec34793549e ]---
> [ 9.973873] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
> Bisect log is attached. Reverting this patch fixes the problem. The problem
> is always seen with this patch applied, and is never seen with this patch
> reverted.
>
> Any idea what might be going on, and how to debug the problem ?

Could you please send me the complete boot log, as well as the
kernel config file please?

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

2021-10-27 14:41:14

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On 10/26/21 7:59 PM, Herbert Xu wrote:
> On Tue, Oct 26, 2021 at 09:33:19AM -0700, Guenter Roeck wrote:
>>
>> I can not explain it, but this patch causes a crash with one of my boot
>> tests (riscv32 with riscv32 virt machine and e1000 network adapter):
>>
>> [ 9.948557] e1000 0000:00:01.0: enabling device (0000 -> 0003)
>> [ 9.968578] Unable to handle kernel paging request at virtual address 9e000000
>> [ 9.969207] Oops [#1]
>> [ 9.969325] Modules linked in:
>> [ 9.969619] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc6-next-20211025 #1
>> [ 9.969983] Hardware name: riscv-virtio,qemu (DT)
>> [ 9.970262] epc : e1000_io_write+0x10/0x1c
>> [ 9.970487] ra : e1000_reset_hw+0xfa/0x312
>> [ 9.970639] epc : c07b3a44 ra : c07b5e4a sp : c258dcf0
>> [ 9.970792] gp : c1d6cfa0 tp : c25b0040 t0 : c1f05b3c
>> [ 9.970941] t1 : 04d6d7d4 t2 : 00001fff s0 : c258dd00
>> [ 9.971091] s1 : c36a9990 a0 : c36a9990 a1 : 9e000000
>> [ 9.971240] a2 : 00000000 a3 : 04000000 a4 : 00000002
>> [ 9.971389] a5 : 9e000000 a6 : 00000000 a7 : 00006000
>> [ 9.971539] s2 : c101b3ec s3 : c23aceb0 s4 : 04140240
>> [ 9.971692] s5 : 00000000 s6 : c14a3550 s7 : c1d72000
>> [ 9.971872] s8 : 00000000 s9 : c36a9000 s10: 00000000
>> [ 9.972037] s11: 00000000 t3 : cb75ee6c t4 : 0000000c
>> [ 9.972200] t5 : 000021cb t6 : c1f017a0
>> [ 9.972336] status: 00000120 badaddr: 9e000000 cause: 0000000f
>> [ 9.972570] [<c07b3a44>] e1000_io_write+0x10/0x1c
>> [ 9.973382] ---[ end trace 49388ec34793549e ]---
>> [ 9.973873] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>>
>> Bisect log is attached. Reverting this patch fixes the problem. The problem
>> is always seen with this patch applied, and is never seen with this patch
>> reverted.
>>
>> Any idea what might be going on, and how to debug the problem ?
>
> Could you please send me the complete boot log, as well as the
> kernel config file please?
>

You should find everything you should need to reproduce the problem,
including a full crash log, at http://server.roeck-us.net/qemu/riscv32/.

Hope this helps,

Guenter

2021-11-02 15:44:14

[permalink] [raw]

Subject: Re: [PATCH] crypto: api - Do not create test larvals if manager is disabled

Hi Herbert,

On Tue, 19 Oct 2021, Herbert Xu wrote:
> On Tue, Oct 05, 2021 at 07:33:28PM -0700, Nathan Chancellor wrote:
>> I assume this is the diff you mean? This does not resolve the issue. My
>> apologies if I am slow to respond, I am on vacation until the middle of
>> next week.
>
> Sorry for the delay. The kernel robot figured out the problem
> for me. It's the crypto_alg_tested call that causes api.c to
> depend on algapi.c. This call is only invoked in the case where
> the crypto manager is turned off. We could instead simply make
> test larvals disappear in that case.
>
> ---8<---
> The delayed boot-time testing patch created a dependency loop
> between api.c and algapi.c because it added a crypto_alg_tested
> call to the former when the crypto manager is disabled.
>
> We could instead avoid creating the test larvals if the crypto
> manager is disabled. This avoids the dependency loop as well
> as saving some unnecessary work, albeit in a very unlikely case.
>
> Reported-by: Nathan Chancellor <[email protected]>
> Reported-by: Naresh Kamboju <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Fixes: adad556efcdd ("crypto: api - Fix built-in testing dependency failures")
> Signed-off-by: Herbert Xu <[email protected]>

Thanks for your patch, which is now commit cad439fc040efe5f
("crypto: api - Do not create test larvals if manager is disabled").

I have bisected a failure to mount the root file system on k210 to this
commit.

Dmesg before/after:

mmcblk0: mmc0:0000 SA04G 3.68 GiB
random: fast init done
mmcblk0: p1
-EXT4-fs (mmcblk0p1): mounted filesystem with ordered data mode. Opts: (null). Quota mode: disabled.
-VFS: Mounted root (ext4 filesystem) readonly on device 179:1.
+EXT4-fs (mmcblk0p1): Cannot load crc32c driver.
+VFS: Cannot open root device "mmcblk0p1" or unknown-block(179,1): error -80
+Please append a correct "root=" boot option; here are the available partitions:
+b300 3858432 mmcblk0
+ driver: mmcblk
+ b301 3854336 mmcblk0p1 00000000-01

> --- a/crypto/algapi.c
> +++ b/crypto/algapi.c
> @@ -216,6 +216,32 @@ void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
> }
> EXPORT_SYMBOL_GPL(crypto_remove_spawns);
>
> +static struct crypto_larval *crypto_alloc_test_larval(struct crypto_alg *alg)
> +{
> + struct crypto_larval *larval;
> +
> + if (!IS_ENABLED(CONFIG_CRYPTO_MANAGER))
> + return NULL;
> +
> + larval = crypto_larval_alloc(alg->cra_name,
> + alg->cra_flags | CRYPTO_ALG_TESTED, 0);
> + if (IS_ERR(larval))
> + return larval;
> +
> + larval->adult = crypto_mod_get(alg);
> + if (!larval->adult) {
> + kfree(larval);
> + return ERR_PTR(-ENOENT);
> + }
> +
> + refcount_set(&larval->alg.cra_refcnt, 1);
> + memcpy(larval->alg.cra_driver_name, alg->cra_driver_name,
> + CRYPTO_MAX_ALG_NAME);
> + larval->alg.cra_priority = alg->cra_priority;
> +
> + return larval;
> +}
> +
> static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
> {
> struct crypto_alg *q;
> @@ -250,31 +276,20 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
> goto err;
> }
>
> - larval = crypto_larval_alloc(alg->cra_name,
> - alg->cra_flags | CRYPTO_ALG_TESTED, 0);
> + larval = crypto_alloc_test_larval(alg);
> if (IS_ERR(larval))
> goto out;
>
> - ret = -ENOENT;
> - larval->adult = crypto_mod_get(alg);
> - if (!larval->adult)
> - goto free_larval;
> -
> - refcount_set(&larval->alg.cra_refcnt, 1);
> - memcpy(larval->alg.cra_driver_name, alg->cra_driver_name,
> - CRYPTO_MAX_ALG_NAME);
> - larval->alg.cra_priority = alg->cra_priority;
> -
> list_add(&alg->cra_list, &crypto_alg_list);
> - list_add(&larval->alg.cra_list, &crypto_alg_list);
> +
> + if (larval)
> + list_add(&larval->alg.cra_list, &crypto_alg_list);
>
> crypto_stats_init(alg);
>
> out:
> return larval;
>
> -free_larval:
> - kfree(larval);
> err:
> larval = ERR_PTR(ret);
> goto out;
> @@ -403,10 +418,11 @@ int crypto_register_alg(struct crypto_alg *alg)
> down_write(&crypto_alg_sem);
> larval = __crypto_register_alg(alg);
> test_started = static_key_enabled(&crypto_boot_test_finished);
> - larval->test_started = test_started;
> + if (!IS_ERR_OR_NULL(larval))
> + larval->test_started = test_started;
> up_write(&crypto_alg_sem);
>
> - if (IS_ERR(larval))
> + if (IS_ERR_OR_NULL(larval))
> return PTR_ERR(larval);
>
> if (test_started)
> @@ -616,8 +632,8 @@ int crypto_register_instance(struct crypto_template *tmpl,
> larval = __crypto_register_alg(&inst->alg);
> if (IS_ERR(larval))
> goto unlock;
> -
> - larval->test_started = true;
> + else if (larval)
> + larval->test_started = true;
>
> hlist_add_head(&inst->list, &tmpl->instances);
> inst->tmpl = tmpl;
> @@ -626,7 +642,7 @@ int crypto_register_instance(struct crypto_template *tmpl,
> up_write(&crypto_alg_sem);
>
> err = PTR_ERR(larval);
> - if (IS_ERR(larval))
> + if (IS_ERR_OR_NULL(larval))
> goto err;
>
> crypto_wait_for_test(larval);
> diff --git a/crypto/api.c b/crypto/api.c
> index ee5991fe11f8..cf0869dd130b 100644
> --- a/crypto/api.c
> +++ b/crypto/api.c
> @@ -167,11 +167,8 @@ void crypto_wait_for_test(struct crypto_larval *larval)
> int err;
>
> err = crypto_probing_notify(CRYPTO_MSG_ALG_REGISTER, larval->adult);
> - if (err != NOTIFY_STOP) {
> - if (WARN_ON(err != NOTIFY_DONE))
> - goto out;
> - crypto_alg_tested(larval->alg.cra_driver_name, 0);
> - }
> + if (WARN_ON_ONCE(err != NOTIFY_STOP))
> + goto out;
>
> err = wait_for_completion_killable(&larval->completion);
> WARN_ON(err);

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-11-04 07:29:59

by Damien Le Moal

[permalink] [raw]

Subject: Re: [PATCH] crypto: api - Do not create test larvals if manager is disabled

On 2021/11/03 0:41, Geert Uytterhoeven wrote:
> Hi Herbert,
>
> On Tue, 19 Oct 2021, Herbert Xu wrote:
>> On Tue, Oct 05, 2021 at 07:33:28PM -0700, Nathan Chancellor wrote:
>>> I assume this is the diff you mean? This does not resolve the issue. My
>>> apologies if I am slow to respond, I am on vacation until the middle of
>>> next week.
>>
>> Sorry for the delay. The kernel robot figured out the problem
>> for me. It's the crypto_alg_tested call that causes api.c to
>> depend on algapi.c. This call is only invoked in the case where
>> the crypto manager is turned off. We could instead simply make
>> test larvals disappear in that case.
>>
>> ---8<---
>> The delayed boot-time testing patch created a dependency loop
>> between api.c and algapi.c because it added a crypto_alg_tested
>> call to the former when the crypto manager is disabled.
>>
>> We could instead avoid creating the test larvals if the crypto
>> manager is disabled. This avoids the dependency loop as well
>> as saving some unnecessary work, albeit in a very unlikely case.
>>
>> Reported-by: Nathan Chancellor <[email protected]>
>> Reported-by: Naresh Kamboju <[email protected]>
>> Reported-by: kernel test robot <[email protected]>
>> Fixes: adad556efcdd ("crypto: api - Fix built-in testing dependency failures")
>> Signed-off-by: Herbert Xu <[email protected]>
>
> Thanks for your patch, which is now commit cad439fc040efe5f
> ("crypto: api - Do not create test larvals if manager is disabled").
>
> I have bisected a failure to mount the root file system on k210 to this
> commit.
>
> Dmesg before/after:
>
> mmcblk0: mmc0:0000 SA04G 3.68 GiB
> random: fast init done
> mmcblk0: p1
> -EXT4-fs (mmcblk0p1): mounted filesystem with ordered data mode. Opts: (null). Quota mode: disabled.
> -VFS: Mounted root (ext4 filesystem) readonly on device 179:1.
> +EXT4-fs (mmcblk0p1): Cannot load crc32c driver.
> +VFS: Cannot open root device "mmcblk0p1" or unknown-block(179,1): error -80

p1 exist as the message 2 lines above shows. And since the mount error is -80
(ELIBBAD), it is really all about crypto. Since the default k210 config compile
everything in-kernel (no modules), it should work. Was crc32c compiled as a
module ? If yes, then the k210 will need to be booted with U-Boot and use a real
initrd, which likely will all end-up in a no memory situation. ext4 in itself
will consume way too much memory...

> +Please append a correct "root=" boot option; here are the available partitions:
> +b300 3858432 mmcblk0
> + driver: mmcblk
> + b301 3854336 mmcblk0p1 00000000-01
>
>> --- a/crypto/algapi.c
>> +++ b/crypto/algapi.c
>> @@ -216,6 +216,32 @@ void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
>> }
>> EXPORT_SYMBOL_GPL(crypto_remove_spawns);
>>
>> +static struct crypto_larval *crypto_alloc_test_larval(struct crypto_alg *alg)
>> +{
>> + struct crypto_larval *larval;
>> +
>> + if (!IS_ENABLED(CONFIG_CRYPTO_MANAGER))
>> + return NULL;
>> +
>> + larval = crypto_larval_alloc(alg->cra_name,
>> + alg->cra_flags | CRYPTO_ALG_TESTED, 0);
>> + if (IS_ERR(larval))
>> + return larval;
>> +
>> + larval->adult = crypto_mod_get(alg);
>> + if (!larval->adult) {
>> + kfree(larval);
>> + return ERR_PTR(-ENOENT);
>> + }
>> +
>> + refcount_set(&larval->alg.cra_refcnt, 1);
>> + memcpy(larval->alg.cra_driver_name, alg->cra_driver_name,
>> + CRYPTO_MAX_ALG_NAME);
>> + larval->alg.cra_priority = alg->cra_priority;
>> +
>> + return larval;
>> +}
>> +
>> static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
>> {
>> struct crypto_alg *q;
>> @@ -250,31 +276,20 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
>> goto err;
>> }
>>
>> - larval = crypto_larval_alloc(alg->cra_name,
>> - alg->cra_flags | CRYPTO_ALG_TESTED, 0);
>> + larval = crypto_alloc_test_larval(alg);
>> if (IS_ERR(larval))
>> goto out;
>>
>> - ret = -ENOENT;
>> - larval->adult = crypto_mod_get(alg);
>> - if (!larval->adult)
>> - goto free_larval;
>> -
>> - refcount_set(&larval->alg.cra_refcnt, 1);
>> - memcpy(larval->alg.cra_driver_name, alg->cra_driver_name,
>> - CRYPTO_MAX_ALG_NAME);
>> - larval->alg.cra_priority = alg->cra_priority;
>> -
>> list_add(&alg->cra_list, &crypto_alg_list);
>> - list_add(&larval->alg.cra_list, &crypto_alg_list);
>> +
>> + if (larval)
>> + list_add(&larval->alg.cra_list, &crypto_alg_list);
>>
>> crypto_stats_init(alg);
>>
>> out:
>> return larval;
>>
>> -free_larval:
>> - kfree(larval);
>> err:
>> larval = ERR_PTR(ret);
>> goto out;
>> @@ -403,10 +418,11 @@ int crypto_register_alg(struct crypto_alg *alg)
>> down_write(&crypto_alg_sem);
>> larval = __crypto_register_alg(alg);
>> test_started = static_key_enabled(&crypto_boot_test_finished);
>> - larval->test_started = test_started;
>> + if (!IS_ERR_OR_NULL(larval))
>> + larval->test_started = test_started;
>> up_write(&crypto_alg_sem);
>>
>> - if (IS_ERR(larval))
>> + if (IS_ERR_OR_NULL(larval))
>> return PTR_ERR(larval);
>>
>> if (test_started)
>> @@ -616,8 +632,8 @@ int crypto_register_instance(struct crypto_template *tmpl,
>> larval = __crypto_register_alg(&inst->alg);
>> if (IS_ERR(larval))
>> goto unlock;
>> -
>> - larval->test_started = true;
>> + else if (larval)
>> + larval->test_started = true;
>>
>> hlist_add_head(&inst->list, &tmpl->instances);
>> inst->tmpl = tmpl;
>> @@ -626,7 +642,7 @@ int crypto_register_instance(struct crypto_template *tmpl,
>> up_write(&crypto_alg_sem);
>>
>> err = PTR_ERR(larval);
>> - if (IS_ERR(larval))
>> + if (IS_ERR_OR_NULL(larval))
>> goto err;
>>
>> crypto_wait_for_test(larval);
>> diff --git a/crypto/api.c b/crypto/api.c
>> index ee5991fe11f8..cf0869dd130b 100644
>> --- a/crypto/api.c
>> +++ b/crypto/api.c
>> @@ -167,11 +167,8 @@ void crypto_wait_for_test(struct crypto_larval *larval)
>> int err;
>>
>> err = crypto_probing_notify(CRYPTO_MSG_ALG_REGISTER, larval->adult);
>> - if (err != NOTIFY_STOP) {
>> - if (WARN_ON(err != NOTIFY_DONE))
>> - goto out;
>> - crypto_alg_tested(larval->alg.cra_driver_name, 0);
>> - }
>> + if (WARN_ON_ONCE(err != NOTIFY_STOP))
>> + goto out;
>>
>> err = wait_for_completion_killable(&larval->completion);
>> WARN_ON(err);
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>

--
Damien Le Moal
Western Digital Research

2021-11-04 07:59:12

[permalink] [raw]

Subject: Re: [PATCH] crypto: api - Do not create test larvals if manager is disabled

Hi Damien,

On Thu, Nov 4, 2021 at 8:29 AM Damien Le Moal <[email protected]> wrote:
> On 2021/11/03 0:41, Geert Uytterhoeven wrote:
> > On Tue, 19 Oct 2021, Herbert Xu wrote:
> >> On Tue, Oct 05, 2021 at 07:33:28PM -0700, Nathan Chancellor wrote:
> >>> I assume this is the diff you mean? This does not resolve the issue. My
> >>> apologies if I am slow to respond, I am on vacation until the middle of
> >>> next week.
> >>
> >> Sorry for the delay. The kernel robot figured out the problem
> >> for me. It's the crypto_alg_tested call that causes api.c to
> >> depend on algapi.c. This call is only invoked in the case where
> >> the crypto manager is turned off. We could instead simply make
> >> test larvals disappear in that case.
> >>
> >> ---8<---
> >> The delayed boot-time testing patch created a dependency loop
> >> between api.c and algapi.c because it added a crypto_alg_tested
> >> call to the former when the crypto manager is disabled.
> >>
> >> We could instead avoid creating the test larvals if the crypto
> >> manager is disabled. This avoids the dependency loop as well
> >> as saving some unnecessary work, albeit in a very unlikely case.
> >>
> >> Reported-by: Nathan Chancellor <[email protected]>
> >> Reported-by: Naresh Kamboju <[email protected]>
> >> Reported-by: kernel test robot <[email protected]>
> >> Fixes: adad556efcdd ("crypto: api - Fix built-in testing dependency failures")
> >> Signed-off-by: Herbert Xu <[email protected]>
> >
> > Thanks for your patch, which is now commit cad439fc040efe5f
> > ("crypto: api - Do not create test larvals if manager is disabled").
> >
> > I have bisected a failure to mount the root file system on k210 to this
> > commit.
> >
> > Dmesg before/after:
> >
> > mmcblk0: mmc0:0000 SA04G 3.68 GiB
> > random: fast init done
> > mmcblk0: p1
> > -EXT4-fs (mmcblk0p1): mounted filesystem with ordered data mode. Opts: (null). Quota mode: disabled.
> > -VFS: Mounted root (ext4 filesystem) readonly on device 179:1.
> > +EXT4-fs (mmcblk0p1): Cannot load crc32c driver.
> > +VFS: Cannot open root device "mmcblk0p1" or unknown-block(179,1): error -80
>
> p1 exist as the message 2 lines above shows. And since the mount error is -80
> (ELIBBAD), it is really all about crypto. Since the default k210 config compile
> everything in-kernel (no modules), it should work. Was crc32c compiled as a
> module ? If yes, then the k210 will need to be booted with U-Boot and use a real
> initrd, which likely will all end-up in a no memory situation. ext4 in itself
> will consume way too much memory...

Everything is built-in, including crc32c. It worked fine, until the commit
referenced.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-11-04 08:06:01

by Damien Le Moal

[permalink] [raw]

Subject: Re: [PATCH] crypto: api - Do not create test larvals if manager is disabled

On 2021/11/04 16:58, Geert Uytterhoeven wrote:
> Hi Damien,
>
> On Thu, Nov 4, 2021 at 8:29 AM Damien Le Moal <[email protected]> wrote:
>> On 2021/11/03 0:41, Geert Uytterhoeven wrote:
>>> On Tue, 19 Oct 2021, Herbert Xu wrote:
>>>> On Tue, Oct 05, 2021 at 07:33:28PM -0700, Nathan Chancellor wrote:
>>>>> I assume this is the diff you mean? This does not resolve the issue. My
>>>>> apologies if I am slow to respond, I am on vacation until the middle of
>>>>> next week.
>>>>
>>>> Sorry for the delay. The kernel robot figured out the problem
>>>> for me. It's the crypto_alg_tested call that causes api.c to
>>>> depend on algapi.c. This call is only invoked in the case where
>>>> the crypto manager is turned off. We could instead simply make
>>>> test larvals disappear in that case.
>>>>
>>>> ---8<---
>>>> The delayed boot-time testing patch created a dependency loop
>>>> between api.c and algapi.c because it added a crypto_alg_tested
>>>> call to the former when the crypto manager is disabled.
>>>>
>>>> We could instead avoid creating the test larvals if the crypto
>>>> manager is disabled. This avoids the dependency loop as well
>>>> as saving some unnecessary work, albeit in a very unlikely case.
>>>>
>>>> Reported-by: Nathan Chancellor <[email protected]>
>>>> Reported-by: Naresh Kamboju <[email protected]>
>>>> Reported-by: kernel test robot <[email protected]>
>>>> Fixes: adad556efcdd ("crypto: api - Fix built-in testing dependency failures")
>>>> Signed-off-by: Herbert Xu <[email protected]>
>>>
>>> Thanks for your patch, which is now commit cad439fc040efe5f
>>> ("crypto: api - Do not create test larvals if manager is disabled").
>>>
>>> I have bisected a failure to mount the root file system on k210 to this
>>> commit.
>>>
>>> Dmesg before/after:
>>>
>>> mmcblk0: mmc0:0000 SA04G 3.68 GiB
>>> random: fast init done
>>> mmcblk0: p1
>>> -EXT4-fs (mmcblk0p1): mounted filesystem with ordered data mode. Opts: (null). Quota mode: disabled.
>>> -VFS: Mounted root (ext4 filesystem) readonly on device 179:1.
>>> +EXT4-fs (mmcblk0p1): Cannot load crc32c driver.
>>> +VFS: Cannot open root device "mmcblk0p1" or unknown-block(179,1): error -80
>>
>> p1 exist as the message 2 lines above shows. And since the mount error is -80
>> (ELIBBAD), it is really all about crypto. Since the default k210 config compile
>> everything in-kernel (no modules), it should work. Was crc32c compiled as a
>> module ? If yes, then the k210 will need to be booted with U-Boot and use a real
>> initrd, which likely will all end-up in a no memory situation. ext4 in itself
>> will consume way too much memory...
>
> Everything is built-in, including crc32c. It worked fine, until the commit
> referenced.

I think I read this backward. I thought one of my k210 patch was wrong... It is
the crypto patch that is at fault here.

Thanks !

>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>

--
Damien Le Moal
Western Digital Research

2021-11-04 12:17:47

[permalink] [raw]

Subject: Re: [PATCH] crypto: api - Do not create test larvals if manager is disabled

On Thu, Nov 04, 2021 at 08:58:16AM +0100, Geert Uytterhoeven wrote:
> Hi Damien,
>
> On Thu, Nov 4, 2021 at 8:29 AM Damien Le Moal <[email protected]> wrote:
> > On 2021/11/03 0:41, Geert Uytterhoeven wrote:
>
> > > Thanks for your patch, which is now commit cad439fc040efe5f
> > > ("crypto: api - Do not create test larvals if manager is disabled").
> > >
> > > I have bisected a failure to mount the root file system on k210 to this
> > > commit.
> > >
> > > Dmesg before/after:
> > >
> > > mmcblk0: mmc0:0000 SA04G 3.68 GiB
> > > random: fast init done
> > > mmcblk0: p1
> > > -EXT4-fs (mmcblk0p1): mounted filesystem with ordered data mode. Opts: (null). Quota mode: disabled.
> > > -VFS: Mounted root (ext4 filesystem) readonly on device 179:1.
> > > +EXT4-fs (mmcblk0p1): Cannot load crc32c driver.
> > > +VFS: Cannot open root device "mmcblk0p1" or unknown-block(179,1): error -80
> >
> > p1 exist as the message 2 lines above shows. And since the mount error is -80
> > (ELIBBAD), it is really all about crypto. Since the default k210 config compile
> > everything in-kernel (no modules), it should work. Was crc32c compiled as a
> > module ? If yes, then the k210 will need to be booted with U-Boot and use a real
> > initrd, which likely will all end-up in a no memory situation. ext4 in itself
> > will consume way too much memory...
>
> Everything is built-in, including crc32c. It worked fine, until the commit
> referenced.

Can someone please send me the Kconfig used in this case?

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

2021-11-04 13:12:25

[permalink] [raw]

Subject: Re: [PATCH] crypto: api - Do not create test larvals if manager is disabled

Hi Herbert,

On Thu, Nov 4, 2021 at 1:16 PM Herbert Xu <[email protected]> wrote:
> On Thu, Nov 04, 2021 at 08:58:16AM +0100, Geert Uytterhoeven wrote:
> > On Thu, Nov 4, 2021 at 8:29 AM Damien Le Moal <[email protected]> wrote:
> > > On 2021/11/03 0:41, Geert Uytterhoeven wrote:
> >
> > > > Thanks for your patch, which is now commit cad439fc040efe5f
> > > > ("crypto: api - Do not create test larvals if manager is disabled").
> > > >
> > > > I have bisected a failure to mount the root file system on k210 to this
> > > > commit.
> > > >
> > > > Dmesg before/after:
> > > >
> > > > mmcblk0: mmc0:0000 SA04G 3.68 GiB
> > > > random: fast init done
> > > > mmcblk0: p1
> > > > -EXT4-fs (mmcblk0p1): mounted filesystem with ordered data mode. Opts: (null). Quota mode: disabled.
> > > > -VFS: Mounted root (ext4 filesystem) readonly on device 179:1.
> > > > +EXT4-fs (mmcblk0p1): Cannot load crc32c driver.
> > > > +VFS: Cannot open root device "mmcblk0p1" or unknown-block(179,1): error -80
> > >
> > > p1 exist as the message 2 lines above shows. And since the mount error is -80
> > > (ELIBBAD), it is really all about crypto. Since the default k210 config compile
> > > everything in-kernel (no modules), it should work. Was crc32c compiled as a
> > > module ? If yes, then the k210 will need to be booted with U-Boot and use a real
> > > initrd, which likely will all end-up in a no memory situation. ext4 in itself
> > > will consume way too much memory...
> >
> > Everything is built-in, including crc32c. It worked fine, until the commit
> > referenced.
>
> Can someone please send me the Kconfig used in this case?

My config is nommu_k210_sdcard_defconfig with the changes below:

diff --git a/arch/riscv/configs/nommu_k210_sdcard_defconfig.orig
b/arch/riscv/configs/nommu_k210_sdcard_defconfig
index 61f887f65419950c..f14ea3803cea5f3d 100644
--- a/arch/riscv/configs/nommu_k210_sdcard_defconfig.orig
+++ b/arch/riscv/configs/nommu_k210_sdcard_defconfig
@@ -1,3 +1,5 @@
+CONFIG_WERROR=y
+CONFIG_LOCALVERSION="-k210"
# CONFIG_CPU_ISOLATION is not set
CONFIG_LOG_BUF_SHIFT=13
CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=12
@@ -21,16 +23,15 @@ CONFIG_EMBEDDED=y
CONFIG_SLOB=y
# CONFIG_MMU is not set
CONFIG_SOC_CANAAN=y
-CONFIG_SOC_CANAAN_K210_DTB_SOURCE="k210_generic"
-CONFIG_MAXPHYSMEM_2GB=y
+CONFIG_SOC_CANAAN_K210_DTB_SOURCE="sipeed_maix_bit"
CONFIG_SMP=y
CONFIG_NR_CPUS=2
CONFIG_CMDLINE="earlycon console=ttySIF0 rootdelay=2 root=/dev/mmcblk0p1 ro"
CONFIG_CMDLINE_FORCE=y
# CONFIG_SECCOMP is not set
# CONFIG_STACKPROTECTOR is not set
-# CONFIG_GCC_PLUGINS is not set
-# CONFIG_BLK_DEV_BSG is not set
+CONFIG_PARTITION_ADVANCED=y
+# CONFIG_EFI_PARTITION is not set
# CONFIG_MQ_IOSCHED_DEADLINE is not set
# CONFIG_MQ_IOSCHED_KYBER is not set
CONFIG_BINFMT_FLAT=y
@@ -39,10 +40,16 @@ CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
# CONFIG_FW_LOADER is not set
# CONFIG_ALLOW_DEV_COREDUMP is not set
+CONFIG_MTD=y
+# CONFIG_MTD_OF_PARTS is not set
+CONFIG_MTD_SPI_NOR=y
# CONFIG_BLK_DEV is not set
-# CONFIG_INPUT is not set
+# CONFIG_INPUT_LEDS is not set
+# CONFIG_INPUT_KEYBOARD is not set
+# CONFIG_INPUT_MOUSE is not set
# CONFIG_SERIO is not set
# CONFIG_VT is not set
+# CONFIG_UNIX98_PTYS is not set
# CONFIG_LEGACY_PTYS is not set
# CONFIG_LDISC_AUTOLOAD is not set
# CONFIG_HW_RANDOM is not set
@@ -52,7 +59,6 @@ CONFIG_I2C_CHARDEV=y
# CONFIG_I2C_HELPER_AUTO is not set
CONFIG_I2C_DESIGNWARE_PLATFORM=y
CONFIG_SPI=y
-# CONFIG_SPI_MEM is not set
CONFIG_SPI_DESIGNWARE=y
CONFIG_SPI_DW_MMIO=y
# CONFIG_GPIO_CDEV_V1 is not set
@@ -61,6 +67,7 @@ CONFIG_GPIO_SIFIVE=y
CONFIG_POWER_RESET=y
CONFIG_POWER_RESET_SYSCON=y
# CONFIG_HWMON is not set
+# CONFIG_HID is not set
# CONFIG_USB_SUPPORT is not set
CONFIG_MMC=y
# CONFIG_PWRSEQ_EMMC is not set
@@ -72,8 +79,9 @@ CONFIG_LEDS_GPIO=y
CONFIG_LEDS_USER=y
# CONFIG_VIRTIO_MENU is not set
# CONFIG_VHOST_MENU is not set
-# CONFIG_SURFACE_PLATFORMS is not set
-CONFIG_EXT2_FS=y
+# CONFIG_NVMEM is not set
+CONFIG_EXT4_FS=y
+# CONFIG_EXT4_USE_FOR_EXT2 is not set
# CONFIG_FILE_LOCKING is not set
# CONFIG_DNOTIFY is not set
# CONFIG_INOTIFY_USER is not set
@@ -82,8 +90,8 @@ CONFIG_LSM="[]"
CONFIG_PRINTK_TIME=y
# CONFIG_SYMBOLIC_ERRNAME is not set
# CONFIG_DEBUG_BUGVERBOSE is not set
-# CONFIG_SECTION_MISMATCH_WARN_ONLY is not set
# CONFIG_FRAME_POINTER is not set
+CONFIG_VMLINUX_MAP=y
# CONFIG_DEBUG_MISC is not set
CONFIG_PANIC_ON_OOPS=y
# CONFIG_SCHED_DEBUG is not set

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-11-04 13:32:31

[permalink] [raw]

Subject: Re: [PATCH] crypto: api - Do not create test larvals if manager is disabled

On Thu, Nov 04, 2021 at 02:11:34PM +0100, Geert Uytterhoeven wrote:
>
> My config is nommu_k210_sdcard_defconfig with the changes below:

Could you send me the actual config? Just in case something weird
happened during the kconfig process and options got flipped from
their default values.

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

2021-11-04 15:19:44

by Ido Schimmel

[permalink] [raw]

Subject: Re: [PATCH] crypto: api - Do not create test larvals if manager is disabled

On Thu, Nov 04, 2021 at 09:30:44PM +0800, Herbert Xu wrote:
> On Thu, Nov 04, 2021 at 02:11:34PM +0100, Geert Uytterhoeven wrote:
> >
> > My config is nommu_k210_sdcard_defconfig with the changes below:
>
> Could you send me the actual config? Just in case something weird
> happened during the kconfig process and options got flipped from
> their default values.

I can confirm Geert's observation. My system boots successfully with
cad439fc040e^, but fails with cad439fc040e. This is the error in the
kernel log:

EXT4-fs (sda3): Cannot load crc32c driver.

Attached my config. I can easily test patches.

Thanks

Attachments:

(No filename) (634.00 B)
config (112.20 kB)
Download all attachments

2021-11-05 08:14:59

[permalink] [raw]

Subject: crypto: api - Fix boot-up crash when crypto manager is disabled

On Thu, Nov 04, 2021 at 05:18:34PM +0200, Ido Schimmel wrote:
>
> Attached my config. I can easily test patches.

Thanks!

Could you all try this patch please?

---8<---
When the crypto manager is disabled, we need to explicitly set
the crypto algorithms' tested status so that they can be used.

Fixes: cad439fc040e ("crypto: api - Do not create test larvals if...")
Reported-by: Geert Uytterhoeven <[email protected]>
Reported-by: Ido Schimmel <[email protected]>
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>

diff --git a/crypto/algapi.c b/crypto/algapi.c
index d379fd91fb7b..a366cb3e8aa1 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -284,6 +284,8 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)

if (larval)
list_add(&larval->alg.cra_list, &crypto_alg_list);
+ else
+ alg->cra_flags |= CRYPTO_ALG_TESTED;

crypto_stats_init(alg);

--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2021-11-05 13:28:00

[permalink] [raw]

Subject: Re: crypto: api - Fix boot-up crash when crypto manager is disabled

On Fri, Nov 05, 2021 at 03:26:08PM +0800, Herbert Xu wrote:
> On Thu, Nov 04, 2021 at 05:18:34PM +0200, Ido Schimmel wrote:
> >
> > Attached my config. I can easily test patches.
>
> Thanks!
>
> Could you all try this patch please?
>
> ---8<---
> When the crypto manager is disabled, we need to explicitly set
> the crypto algorithms' tested status so that they can be used.
>
> Fixes: cad439fc040e ("crypto: api - Do not create test larvals if...")
> Reported-by: Geert Uytterhoeven <[email protected]>
> Reported-by: Ido Schimmel <[email protected]>
> Reported-by: Guenter Roeck <[email protected]>
> Signed-off-by: Herbert Xu <[email protected]>

Tested-by: Guenter Roeck <[email protected]>

>
> diff --git a/crypto/algapi.c b/crypto/algapi.c
> index d379fd91fb7b..a366cb3e8aa1 100644
> --- a/crypto/algapi.c
> +++ b/crypto/algapi.c
> @@ -284,6 +284,8 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
>
> if (larval)
> list_add(&larval->alg.cra_list, &crypto_alg_list);
> + else
> + alg->cra_flags |= CRYPTO_ALG_TESTED;
>
> crypto_stats_init(alg);
>
> --
> Email: Herbert Xu <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2021-11-05 14:34:35

by Ido Schimmel

[permalink] [raw]

Subject: Re: crypto: api - Fix boot-up crash when crypto manager is disabled

2021-11-05 19:29:29

[permalink] [raw]

Subject: Re: crypto: api - Fix boot-up crash when crypto manager is disabled

Hi Herbert,

On Fri, Nov 5, 2021 at 8:26 AM Herbert Xu <[email protected]> wrote:
> Could you all try this patch please?
>
> ---8<---
> When the crypto manager is disabled, we need to explicitly set
> the crypto algorithms' tested status so that they can be used.
>
> Fixes: cad439fc040e ("crypto: api - Do not create test larvals if...")
> Reported-by: Geert Uytterhoeven <[email protected]>
> Reported-by: Ido Schimmel <[email protected]>
> Reported-by: Guenter Roeck <[email protected]>
> Signed-off-by: Herbert Xu <[email protected]>

Thanks!

Tested-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-11-06 10:44:52

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Tue, Oct 26, 2021 at 09:33:19AM -0700, Guenter Roeck wrote:
> Hi,
>
> On Fri, Sep 17, 2021 at 08:26:19AM +0800, Herbert Xu wrote:
> > When complex algorithms that depend on other algorithms are built
> > into the kernel, the order of registration must be done such that
> > the underlying algorithms are ready before the ones on top are
> > registered. As otherwise they would fail during the self-test
> > which is required during registration.
> >
> > In the past we have used subsystem initialisation ordering to
> > guarantee this. The number of such precedence levels are limited
> > and they may cause ripple effects in other subsystems.
> >
> > This patch solves this problem by delaying all self-tests during
> > boot-up for built-in algorithms. They will be tested either when
> > something else in the kernel requests for them, or when we have
> > finished registering all built-in algorithms, whichever comes
> > earlier.
> >
> > Reported-by: Vladis Dronov <[email protected]>
> > Signed-off-by: Herbert Xu <[email protected]>
> >
>
> I can not explain it, but this patch causes a crash with one of my boot
> tests (riscv32 with riscv32 virt machine and e1000 network adapter):
>
> [ 9.948557] e1000 0000:00:01.0: enabling device (0000 -> 0003)

Does this still occur with the latest patch I sent yesterday?

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

2021-11-06 21:04:17

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On 11/5/21 8:47 PM, Herbert Xu wrote:
> On Tue, Oct 26, 2021 at 09:33:19AM -0700, Guenter Roeck wrote:
>> Hi,
>>
>> On Fri, Sep 17, 2021 at 08:26:19AM +0800, Herbert Xu wrote:
>>> When complex algorithms that depend on other algorithms are built
>>> into the kernel, the order of registration must be done such that
>>> the underlying algorithms are ready before the ones on top are
>>> registered. As otherwise they would fail during the self-test
>>> which is required during registration.
>>>
>>> In the past we have used subsystem initialisation ordering to
>>> guarantee this. The number of such precedence levels are limited
>>> and they may cause ripple effects in other subsystems.
>>>
>>> This patch solves this problem by delaying all self-tests during
>>> boot-up for built-in algorithms. They will be tested either when
>>> something else in the kernel requests for them, or when we have
>>> finished registering all built-in algorithms, whichever comes
>>> earlier.
>>>
>>> Reported-by: Vladis Dronov <[email protected]>
>>> Signed-off-by: Herbert Xu <[email protected]>
>>>
>>
>> I can not explain it, but this patch causes a crash with one of my boot
>> tests (riscv32 with riscv32 virt machine and e1000 network adapter):
>>
>> [ 9.948557] e1000 0000:00:01.0: enabling device (0000 -> 0003)
>
> Does this still occur with the latest patch I sent yesterday?
>

No, I don't see that problem anymore, neither in mainline with your
patch applied nor in the latest -next with your patch applied.

Guenter

2021-12-22 10:23:18

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

Hello,

I didn't find a more suitable mail to respond to ...

On Sat, Nov 06, 2021 at 07:55:48AM -0700, Guenter Roeck wrote:
> On 11/5/21 8:47 PM, Herbert Xu wrote:
> > On Tue, Oct 26, 2021 at 09:33:19AM -0700, Guenter Roeck wrote:
> > > Hi,
> > >
> > > On Fri, Sep 17, 2021 at 08:26:19AM +0800, Herbert Xu wrote:
> > > > When complex algorithms that depend on other algorithms are built
> > > > into the kernel, the order of registration must be done such that
> > > > the underlying algorithms are ready before the ones on top are
> > > > registered. As otherwise they would fail during the self-test
> > > > which is required during registration.
> > > >
> > > > In the past we have used subsystem initialisation ordering to
> > > > guarantee this. The number of such precedence levels are limited
> > > > and they may cause ripple effects in other subsystems.
> > > >
> > > > This patch solves this problem by delaying all self-tests during
> > > > boot-up for built-in algorithms. They will be tested either when
> > > > something else in the kernel requests for them, or when we have
> > > > finished registering all built-in algorithms, whichever comes
> > > > earlier.
> > > >
> > > > Reported-by: Vladis Dronov <[email protected]>
> > > > Signed-off-by: Herbert Xu <[email protected]>
> > > >
> > >
> > > I can not explain it, but this patch causes a crash with one of my boot
> > > tests (riscv32 with riscv32 virt machine and e1000 network adapter):
> > >
> > > [ 9.948557] e1000 0000:00:01.0: enabling device (0000 -> 0003)
> >
> > Does this still occur with the latest patch I sent yesterday?
> >
>
> No, I don't see that problem anymore, neither in mainline with your
> patch applied nor in the latest -next with your patch applied.

I still experience a problem with the patch that got
adad556efcdd42a1d9e060cbe5f6161cccf1fa28 in v5.16-rc1. I saw there are
two commit fixing this one (

cad439fc040e crypto: api - Do not create test larvals if manager is disabled
e42dff467ee6 crypto: api - Export crypto_boot_test_finished

) but I still encounter the following on 2f47a9a4dfa3:

[ 0.000000] Booting Linux on physical CPU 0x0
[ 0.000000] Linux version 5.16.0-rc6-00031-g2f47a9a4dfa3 (ukl@dude) (arm-v7a-linux-gnueabihf-gcc (OSELAS.Toolchain-2018.12.0 8-20181130) 8.2.1 20181130, GNU ld (GNU Binutils) 2.31.1) #17 SMP PREEMPT Wed Dec 22 10:51:46 CET 2021
[ 0.000000] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=10c5387d
[ 0.000000] CPU: div instructions available: patching division code
[ 0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[ 0.000000] OF: fdt: Machine model: STMicroelectronics STM32MP157C eval daughter on eval mother
[ 0.000000] Memory policy: Data cache writealloc
[ 0.000000] Reserved memory: created DMA memory pool at 0x10000000, size 0 MiB
[ 0.000000] OF: reserved mem: initialized node mcuram2@10000000, compatible id shared-dma-pool
[ 0.000000] Reserved memory: created DMA memory pool at 0x10040000, size 0 MiB
[ 0.000000] OF: reserved mem: initialized node vdev0vring0@10040000, compatible id shared-dma-pool
[ 0.000000] Reserved memory: created DMA memory pool at 0x10041000, size 0 MiB
[ 0.000000] OF: reserved mem: initialized node vdev0vring1@10041000, compatible id shared-dma-pool
[ 0.000000] Reserved memory: created DMA memory pool at 0x10042000, size 0 MiB
[ 0.000000] OF: reserved mem: initialized node vdev0buffer@10042000, compatible id shared-dma-pool
[ 0.000000] Reserved memory: created DMA memory pool at 0x30000000, size 0 MiB
[ 0.000000] OF: reserved mem: initialized node mcuram@30000000, compatible id shared-dma-pool
[ 0.000000] Reserved memory: created DMA memory pool at 0x38000000, size 0 MiB
[ 0.000000] OF: reserved mem: initialized node retram@38000000, compatible id shared-dma-pool
[ 0.000000] cma: Reserved 16 MiB at 0xfe800000
[ 0.000000] Zone ranges:
[ 0.000000] DMA [mem 0x00000000c0000000-0x00000000e7ffffff]
[ 0.000000] Normal empty
[ 0.000000] HighMem [mem 0x00000000e8000000-0x00000000ffffefff]
[ 0.000000] Movable zone start for each node
[ 0.000000] Early memory node ranges
[ 0.000000] node 0: [mem 0x00000000c0000000-0x00000000e7ffffff]
[ 0.000000] node 0: [mem 0x00000000e8000000-0x00000000efffffff]
[ 0.000000] node 0: [mem 0x00000000f0000000-0x00000000ffffefff]
[ 0.000000] Initmem setup node 0 [mem 0x00000000c0000000-0x00000000ffffefff]
[ 0.000000] psci: probing for conduit method from DT.
[ 0.000000] psci: PSCIv1.1 detected in firmware.
[ 0.000000] psci: Using standard PSCI v0.2 function IDs
[ 0.000000] psci: MIGRATE_INFO_TYPE not supported.
[ 0.000000] psci: SMC Calling Convention v1.2
[ 0.000000] percpu: Embedded 16 pages/cpu s35148 r8192 d22196 u65536
[ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 260703
[ 0.000000] Kernel command line: root=/dev/nfs nfsroot=192.168.23.4:/home/ukl/nfsroot/stm32mp157c-ev1,v3,tcp ip=dhcp console=ttySTM0,115200n8 rootwait rw
[ 0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes, linear)
[ 0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes, linear)
[ 0.000000] mem auto-init: stack:off, heap alloc:off, heap free:off
[ 0.000000] Memory: 874968K/1048572K available (9216K kernel code, 1064K rwdata, 3368K rodata, 1024K init, 200K bss, 157220K reserved, 16384K cma-reserved, 245756K highmem)
[ 0.000000] random: get_random_u32 called from cache_random_seq_create+0x84/0x15c with crng_init=0
[ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1
[ 0.000000] ftrace: allocating 34278 entries in 67 pages
[ 0.000000] ftrace: allocated 67 pages with 3 groups
[ 0.000000] trace event string verifier disabled
[ 0.000000] rcu: Preemptible hierarchical RCU implementation.
[ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
[ 0.000000] Trampoline variant of Tasks RCU enabled.
[ 0.000000] Rude variant of Tasks RCU enabled.
[ 0.000000] Tracing variant of Tasks RCU enabled.
[ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 10 jiffies.
[ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=2
[ 0.000000] NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
[ 0.000000] arch_timer: cp15 timer(s) running at 24.00MHz (virt).
[ 0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x588fe9dc0, max_idle_ns: 440795202592 ns
[ 0.000002] sched_clock: 56 bits at 24MHz, resolution 41ns, wraps every 4398046511097ns
[ 0.000031] Switching to timer-based delay loop, resolution 41ns
[ 0.006510] Calibrating delay loop (skipped), value calculated using timer frequency.. 48.00 BogoMIPS (lpj=240000)
[ 0.006551] pid_max: default: 32768 minimum: 301
[ 0.006906] Mount-cache hash table entries: 2048 (order: 1, 8192 bytes, linear)
[ 0.006943] Mountpoint-cache hash table entries: 2048 (order: 1, 8192 bytes, linear)
[ 0.008536] CPU: Testing write buffer coherency: ok
[ 0.009012] CPU0: update cpu_capacity 1024
[ 0.009041] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
[ 0.010632] Setting up static identity map for 0xc0100000 - 0xc0100060
[ 0.010866] rcu: Hierarchical SRCU implementation.
[ 0.012500] smp: Bringing up secondary CPUs ...
[ 0.013639] CPU1: update cpu_capacity 1024
[ 0.013660] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
[ 0.013885] smp: Brought up 1 node, 2 CPUs
[ 0.013929] SMP: Total of 2 processors activated (96.00 BogoMIPS).
[ 0.013948] CPU: All CPU(s) started in SVC mode.
[ 0.014920] devtmpfs: initialized
[ 0.040428] VFP support v0.3: implementor 41 architecture 2 part 30 variant 7 rev 5
[ 0.040723] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[ 0.040766] futex hash table entries: 512 (order: 3, 32768 bytes, linear)
[ 0.041935] pinctrl core: initialized pinctrl subsystem
[ 0.043866] NET: Registered PF_NETLINK/PF_ROUTE protocol family
[ 0.047307] DMA: preallocated 256 KiB pool for atomic coherent allocations
[ 0.050013] thermal_sys: Registered thermal governor 'step_wise'
[ 0.050438] cpuidle: using governor ladder
[ 0.050517] cpuidle: using governor menu
[ 0.051016] hw-breakpoint: found 5 (+1 reserved) breakpoint and 4 watchpoint registers.
[ 0.051048] hw-breakpoint: maximum watchpoint size is 8 bytes.
[ 0.051639] Serial: AMBA PL011 UART driver
[ 0.084163] /soc/interrupt-controller@5000d000: bank0
[ 0.084225] /soc/interrupt-controller@5000d000: bank1
[ 0.084257] /soc/interrupt-controller@5000d000: bank2
[ 0.099885] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOA bank added
[ 0.100856] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOB bank added
[ 0.101680] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOC bank added
[ 0.102428] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOD bank added
[ 0.103191] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOE bank added
[ 0.103985] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOF bank added
[ 0.104776] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOG bank added
[ 0.105569] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOH bank added
[ 0.106332] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOI bank added
[ 0.107158] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOJ bank added
[ 0.107896] stm32mp157-pinctrl soc:pin-controller@50002000: GPIOK bank added
[ 0.108167] stm32mp157-pinctrl soc:pin-controller@50002000: Pinctrl STM32 initialized
[ 0.112296] stm32mp157-pinctrl soc:pin-controller-z@54004000: GPIOZ bank added
[ 0.112355] stm32mp157-pinctrl soc:pin-controller-z@54004000: Pinctrl STM32 initialized
[ 0.115621] platform 5a000000.dsi: Fixing up cyclic dependency with 5a001000.display-controller
[ 0.134041] cryptd: max_cpu_qlen set to 1000
[ 0.137839] stm32-dma 48000000.dma-controller: STM32 DMA driver registered
[ 0.139723] stm32-dma 48001000.dma-controller: STM32 DMA driver registered
[ 0.143877] stm32-mdma 58000000.dma-controller: STM32 MDMA driver registered
[ 0.147444] usbcore: registered new interface driver usbfs
[ 0.147563] usbcore: registered new interface driver hub
[ 0.147643] usbcore: registered new device driver usb
[ 0.148661] pps_core: LinuxPPS API ver. 1 registered
[ 0.148682] pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <[email protected]>
[ 0.148722] PTP clock support registered
[ 0.151497] clocksource: Switched to clocksource arch_sys_counter
[ 0.224527] NET: Registered PF_INET protocol family
[ 0.224891] IP idents hash table entries: 16384 (order: 5, 131072 bytes, linear)
[ 0.226763] tcp_listen_portaddr_hash hash table entries: 512 (order: 0, 6144 bytes, linear)
[ 0.226838] TCP established hash table entries: 8192 (order: 3, 32768 bytes, linear)
[ 0.226975] TCP bind hash table entries: 8192 (order: 4, 65536 bytes, linear)
[ 0.227163] TCP: Hash tables configured (established 8192 bind 8192)
[ 0.227345] UDP hash table entries: 512 (order: 2, 16384 bytes, linear)
[ 0.227432] UDP-Lite hash table entries: 512 (order: 2, 16384 bytes, linear)
[ 0.227773] NET: Registered PF_UNIX/PF_LOCAL protocol family
[ 0.228694] RPC: Registered named UNIX socket transport module.
[ 0.228733] RPC: Registered udp transport module.
[ 0.228747] RPC: Registered tcp transport module.
[ 0.228760] RPC: Registered tcp NFSv4.1 backchannel transport module.
[ 0.228779] NET: Registered PF_XDP protocol family
[ 0.229889] hw perfevents: enabled with armv7_cortex_a7 PMU driver, 5 counters available
[ 0.232522] workingset: timestamp_bits=14 max_order=18 bucket_order=4
[ 0.247322] NFS: Registering the id_resolver key type
[ 0.247411] Key type id_resolver registered
[ 0.247428] Key type id_legacy registered
[ 0.247796] 9p: Installing v9fs 9p2000 file system support
[ 0.248584] bounce: pool size: 64 pages
[ 0.257261] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[ 0.259560] STM32 USART driver initialized
[ 0.260412] stm32-usart 40010000.serial: interrupt mode for rx (no dma)
[ 0.260447] stm32-usart 40010000.serial: interrupt mode for tx (no dma)
[ 0.260484] 40010000.serial: ttySTM0 at MMIO 0x40010000 (irq = 73, base_baud = 4000000) is a stm32-usart
[ 1.174613] printk: console [ttySTM0] enabled
[ 1.202878] loop: module loaded
[ 1.203635] random: fast init done
[ 1.209179] wireguard: WireGuard 1.0.0 loaded. See http://www.wireguard.com for information.
[ 1.211339] random: crng init done
[ 1.215888] wireguard: Copyright (C) 2015-2019 Jason A. Donenfeld <[email protected]>. All Rights Reserved.
[ 1.230257] libphy: Fixed MDIO Bus: probed
[ 1.236782] stm32-dwmac 5800a000.ethernet: IRQ eth_wake_irq not found
[ 1.241840] stm32-dwmac 5800a000.ethernet: IRQ eth_lpi not found
[ 1.248794] stm32-dwmac 5800a000.ethernet: User ID: 0x40, Synopsys ID: 0x42
[ 1.254847] stm32-dwmac 5800a000.ethernet: DWMAC4/5
[ 1.259691] stm32-dwmac 5800a000.ethernet: DMA HW capability register supported
[ 1.267024] stm32-dwmac 5800a000.ethernet: RX Checksum Offload Engine supported
[ 1.274329] stm32-dwmac 5800a000.ethernet: TX Checksum insertion supported
[ 1.281165] stm32-dwmac 5800a000.ethernet: Wake-Up On Lan supported
[ 1.287450] stm32-dwmac 5800a000.ethernet: TSO supported
[ 1.292752] stm32-dwmac 5800a000.ethernet: Enable RX Mitigation via HW Watchdog Timer
[ 1.300560] stm32-dwmac 5800a000.ethernet: Enabled L3L4 Flow TC (entries=2)
[ 1.307548] stm32-dwmac 5800a000.ethernet: Enabled RFS Flow TC (entries=8)
[ 1.314422] stm32-dwmac 5800a000.ethernet: TSO feature enabled
[ 1.320208] stm32-dwmac 5800a000.ethernet: Using 32 bits DMA width
[ 1.327329] libphy: stmmac: probed
[ 1.335104] usbcore: registered new interface driver asix
[ 1.339123] usbcore: registered new interface driver ax88179_178a
[ 1.345392] usbcore: registered new interface driver smsc95xx
[ 1.351931] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[ 1.357436] ehci-omap: OMAP-EHCI Host Controller driver
[ 1.362882] ehci-atmel: EHCI Atmel driver
[ 1.369108] stm32_rtc 5c004000.rtc: IRQ index 1 not found
[ 1.373092] stm32_rtc 5c004000.rtc: alarm can't wake up the system: -6
[ 1.380262] stm32_rtc 5c004000.rtc: registered as rtc0
[ 1.384811] stm32_rtc 5c004000.rtc: setting system clock to 2000-01-01T02:37:57 UTC (946694277)
[ 1.393860] stm32_rtc 5c004000.rtc: Date/Time must be initialized
[ 1.399487] stm32_rtc 5c004000.rtc: registered rev:1.2
[ 1.404985] i2c_dev: i2c /dev entries driver
[ 1.430431] i2c 0-003c: Fixing up cyclic dependency with 4c006000.dcmi
[ 1.437252] stm32f7-i2c 40013000.i2c: STM32F7 I2C-0 bus adapter
[ 1.463227] stm32f7-i2c 40015000.i2c: STM32F7 I2C-1 bus adapter
[ 1.493652] stpmic1 2-0033: PMIC Chip Version: 0x10
[ 1.514417] vdda: Bringing 1800000uV into 2900000-2900000uV
[ 1.520804] v2v8: Bringing 1800000uV into 2800000-2800000uV
[ 1.534135] v1v8: Bringing 1000000uV into 1800000-1800000uV
[ 1.547247] stm32f7-i2c 5c002000.i2c: STM32F7 I2C-2 bus adapter
[ 1.555103] stm_thermal 50028000.thermal: stm_thermal_probe: Driver initialized successfully
[ 1.566947] mmci-pl18x 58005000.mmc: Got CD GPIO
[ 1.571295] mmci-pl18x 58005000.mmc: mmc0: PL180 manf 53 rev2 at 0x58005000 irq 59,0 (pio)
[ 1.609532] mmci-pl18x 58007000.mmc: mmc1: PL180 manf 53 rev2 at 0x58007000 irq 60,0 (pio)
[ 1.650912] sdhci: Secure Digital Host Controller Interface driver
[ 1.655771] sdhci: Copyright(c) Pierre Ossman
[ 1.660461] sdhci-pltfm: SDHCI platform and OF driver helper
[ 1.669007] ledtrig-cpu: registered to indicate activity on CPUs
[ 1.673999] SMCCC: SOC_ID: ARCH_SOC_ID not implemented, skipping ....
[ 1.682512] stm32-ipcc 4c001000.mailbox: ipcc rev:1.0 enabled, 6 chans, proc 0
[ 1.689646] stm32-rproc 10000000.m4: wdg irq registered
[ 1.700148] mmc0: new ultra high speed DDR50 SDHC card at address 0001
[ 1.706661] remoteproc remoteproc0: m4 is available
[ 1.717321] Initializing XFRM netlink socket
[ 1.720971] NET: Registered PF_INET6 protocol family
[ 1.726295] mmcblk0: mmc0:0001 00000 7.44 GiB
[ 1.730578] Segment Routing with IPv6
[ 1.733350] In-situ OAM (IOAM) with IPv6
[ 1.737435] NET: Registered PF_PACKET protocol family
[ 1.742266] NET: Registered PF_KEY protocol family
[ 1.747283] 9pnet: Installing 9P2000 support
[ 1.751319] Key type dns_resolver registered
[ 1.755779] Registering SWP/SWPB emulation handler
[ 1.762484] Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP ARM
[ 1.768451] Modules linked in:
[ 1.771491] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.16.0-rc6-00031-g2f47a9a4dfa3 #17
[ 1.779575] Hardware name: STM32 (Device Tree Support)
[ 1.784706] PC is at crypto_unregister_alg+0xfc/0x104
[ 1.789748] LR is at 0x0
[ 1.792269] pc : [<c04b7008>] lr : [<00000000>] psr: 20000113
[ 1.798529] sp : c18c9ea0 ip : 00000000 fp : c0e58858
[ 1.803746] r10: 00000008 r9 : c18eb400 r8 : c192b600
[ 1.808963] r7 : c100b41c r6 : c18c9eac r5 : c0f04ec8 r4 : c1b91e80
[ 1.815484] r3 : 00000002 r2 : ffffffff r1 : 00000001 r0 : c0f9e088
[ 1.822006] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 1.829138] Control: 10c5387d Table: c000406a DAC: 00000051
[ 1.834875] Register r0 information: non-slab/vmalloc memory
[ 1.840527] Register r1 information: non-paged memory
[ 1.845570] Register r2 information: non-paged memory
[ 1.850613] Register r3 information: non-paged memory
[ 1.855657] Register r4 information: slab kmalloc-512 start c1b91e00 pointer offset 128 size 512
[ 1.864440] Register r5 information: non-slab/vmalloc memory
[ 1.870092] Register r6 information: non-slab/vmalloc memory
[ 1.875745] Register r7 information: non-slab/vmalloc memory
[ 1.881398] Register r8 information: slab task_struct start c192b600 pointer offset 0
[ 1.889224] Register r9 information: slab kmalloc-128 start c18eb400 pointer offset 0 size 128
[ 1.897834] Register r10 information: non-paged memory
[ 1.902966] Register r11 information: non-slab/vmalloc memory
[ 1.908706] Register r12 information: NULL pointer
[ 1.913489] Process swapper/0 (pid: 1, stack limit = 0xb3b87711)
[ 1.919490] Stack: (0xc18c9ea0 to 0xc18ca000)
[ 1.923847] 9ea0: c192b600 c0f0b16a 00000008 c18c9eac c18c9eac 0cd60047 c1b91e00 c100b430
[ 1.932021] 9ec0: 00000001 c04c3864 c100b420 c011e448 c0f0b240 fffffffe 00000001 c0e083e0
[ 1.940195] 9ee0: c0ff43a0 c0f04ec8 c0e08360 00000000 c192b600 c0101fa0 c019c37c c0e00424
[ 1.948368] 9f00: c0ffc000 00000000 00000007 c0ce0378 c0c44b28 00000000 00000000 c0f04ec8
[ 1.956542] 9f20: c0c5eae8 c0c4e8b8 37320000 c18eb476 00000000 0cd60047 c0f478c0 00000008
[ 1.964715] 9f40: c0d48594 0cd60047 c0e9581c c0d48594 c100b000 c0e58838 000000b4 c0e012f4
[ 1.972888] 9f60: 00000007 00000007 00000000 c0e00424 c0e58808 c0e00424 00000000 00004ec0
[ 1.981062] 9f80: c09eb450 00000000 00000000 00000000 00000000 00000000 00000000 c09eb46c
[ 1.989236] 9fa0: 00000000 c09eb450 00000000 c0100148 00000000 00000000 00000000 00000000
[ 1.997411] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 2.005586] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 2.013769] [<c04b7008>] (crypto_unregister_alg) from [<c04c3864>] (simd_skcipher_free+0x18/0x24)
[ 2.022629] [<c04c3864>] (simd_skcipher_free) from [<c011e448>] (aes_exit+0x28/0x44)
[ 2.030367] [<c011e448>] (aes_exit) from [<c0e083e0>] (aes_init+0x80/0x9c)
[ 2.037238] [<c0e083e0>] (aes_init) from [<c0101fa0>] (do_one_initcall+0x50/0x25c)
[ 2.044802] [<c0101fa0>] (do_one_initcall) from [<c0e012f4>] (kernel_init_freeable+0x21c/0x278)
[ 2.053497] [<c0e012f4>] (kernel_init_freeable) from [<c09eb46c>] (kernel_init+0x1c/0x138)
[ 2.061760] [<c09eb46c>] (kernel_init) from [<c0100148>] (ret_from_fork+0x14/0x2c)
[ 2.069322] Exception stack(0xc18c9fb0 to 0xc18c9ff8)
[ 2.074370] 9fa0: 00000000 00000000 00000000 00000000
[ 2.082548] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 2.090721] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 2.097335] Code: e3a02009 e30011ca eb14a2e2 eaffffec (e7f001f2)
[ 2.103429] ---[ end trace 78f3561a8b67f754 ]---
[ 2.108290] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 2.115684] CPU1: stopping
[ 2.118373] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G D 5.16.0-rc6-00031-g2f47a9a4dfa3 #17
[ 2.127848] Hardware name: STM32 (Device Tree Support)
[ 2.132986] [<c0110d2c>] (unwind_backtrace) from [<c010c81c>] (show_stack+0x18/0x1c)
[ 2.140721] [<c010c81c>] (show_stack) from [<c09e5360>] (dump_stack_lvl+0x40/0x4c)
[ 2.148287] [<c09e5360>] (dump_stack_lvl) from [<c010e8b8>] (do_handle_IPI+0x32c/0x354)
[ 2.156286] [<c010e8b8>] (do_handle_IPI) from [<c010e900>] (ipi_handler+0x20/0x28)
[ 2.163850] [<c010e900>] (ipi_handler) from [<c019813c>] (handle_percpu_devid_irq+0xa8/0x244)
[ 2.172372] [<c019813c>] (handle_percpu_devid_irq) from [<c0191ce0>] (generic_handle_domain_irq+0x4c/0x90)
[ 2.182025] [<c0191ce0>] (generic_handle_domain_irq) from [<c0536310>] (gic_handle_irq+0x7c/0x90)
[ 2.190895] [<c0536310>] (gic_handle_irq) from [<c09eaf04>] (generic_handle_arch_irq+0x58/0x78)
[ 2.199588] [<c09eaf04>] (generic_handle_arch_irq) from [<c0100b10>] (__irq_svc+0x50/0x80)
[ 2.207848] Exception stack(0xc191ff48 to 0xc191ff90)
[ 2.212899] ff40: 000003ba c0c5eae8 00000000 c011a6a0 c0ff4fc0 c0f04f58
[ 2.221073] ff60: 00000002 00000000 00000000 c0cf2868 c1946c00 c0e9ce38 c0f03d00 c191ff98
[ 2.229244] ff80: c01091b0 c01091b4 60000113 ffffffff
[ 2.234286] [<c0100b10>] (__irq_svc) from [<c01091b4>] (arch_cpu_idle+0x40/0x44)
[ 2.241676] [<c01091b4>] (arch_cpu_idle) from [<c09f2620>] (default_idle_call.part.2+0x2c/0x120)
[ 2.250458] [<c09f2620>] (default_idle_call.part.2) from [<c016ae58>] (do_idle+0x260/0x290)
[ 2.258806] [<c016ae58>] (do_idle) from [<c016b18c>] (cpu_startup_entry+0x20/0x24)
[ 2.266369] [<c016b18c>] (cpu_startup_entry) from [<c0101674>] (__enableNOTICE: CPU: STM32MP157AAA Rev.B

Bisection identified adad556efcdd42a1d9e060cbe5f6161cccf1fa28:

# bad: [fa55b7dcdc43c1aa1ba12bca9d2dd4318c2a0dbf] Linux 5.16-rc1
# good: [8bb7eca972ad531c9b149c0a51ab43a417385813] Linux 5.15
git bisect start 'v5.16-rc1' 'v5.15'
# bad: [313b6ffc8e90173f1709b2f4bf9d30c4730a1dde] Merge tag 'linux-kselftest-kunit-5.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest
git bisect bad 313b6ffc8e90173f1709b2f4bf9d30c4730a1dde
# good: [84882cf72cd774cf16fd338bdbf00f69ac9f9194] Revert "net: avoid double accounting for pure zerocopy skbs"
git bisect good 84882cf72cd774cf16fd338bdbf00f69ac9f9194
# good: [79ef0c00142519bc34e1341447f3797436cc48bf] Merge tag 'trace-v5.16' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace
git bisect good 79ef0c00142519bc34e1341447f3797436cc48bf
# good: [0f3d2b680444d5697650b5529c9e749acbf7371f] drm/amdkfd: protect raven_device_info with KFD_SUPPORT_IOMMU_V2
git bisect good 0f3d2b680444d5697650b5529c9e749acbf7371f
# bad: [a64a325bf6313aa5cde7ecd691927e92892d1b7f] Merge tag 'afs-next-20211102' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs
git bisect bad a64a325bf6313aa5cde7ecd691927e92892d1b7f
# good: [52cf891d8dbd7592261fa30f373410b97f22b76c] Merge tag 'kvm-riscv-5.16-2' of https://github.com/kvm-riscv/linux into HEAD
git bisect good 52cf891d8dbd7592261fa30f373410b97f22b76c
# bad: [bfc484fe6abba4b89ec9330e0e68778e2a9856b2] Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
git bisect bad bfc484fe6abba4b89ec9330e0e68778e2a9856b2
# good: [d2fac0afe89fe30c39eaa98dda71f7c4cea190c2] Merge tag 'audit-pr-20211101' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit
git bisect good d2fac0afe89fe30c39eaa98dda71f7c4cea190c2
# bad: [183b60e005975d3c84c22199ca64a9221e620fb6] crypto: hisilicon/qm - modify the uacce mode check
git bisect bad 183b60e005975d3c84c22199ca64a9221e620fb6
# good: [0e64dcd7c94b94f90b820bfbe57bbcea8bf21545] crypto: qat - remove unmatched CPU affinity to cluster IRQ
git bisect good 0e64dcd7c94b94f90b820bfbe57bbcea8bf21545
# good: [f20311cc9c58052e0b215013046cbf390937910c] crypto: caam - disable pkc for non-E SoCs
git bisect good f20311cc9c58052e0b215013046cbf390937910c
# bad: [f7324d4ba9e846e96ac85fbe74afe3fbdacf3b75] hwrng: meson - Improve error handling for core clock
git bisect bad f7324d4ba9e846e96ac85fbe74afe3fbdacf3b75
# good: [7c5329697ed4e0e1bf9a4e4fc9f0053f2f58935d] crypto: marvell/cesa - drop unneeded MODULE_ALIAS
git bisect good 7c5329697ed4e0e1bf9a4e4fc9f0053f2f58935d
# bad: [adad556efcdd42a1d9e060cbe5f6161cccf1fa28] crypto: api - Fix built-in testing dependency failures
git bisect bad adad556efcdd42a1d9e060cbe5f6161cccf1fa28
# first bad commit: [adad556efcdd42a1d9e060cbe5f6161cccf1fa28] crypto: api - Fix built-in testing dependency failures

$ grep CRYPTO .config
CONFIG_ARM_CRYPTO=y
# CONFIG_CRYPTO_SHA1_ARM is not set
# CONFIG_CRYPTO_SHA1_ARM_NEON is not set
# CONFIG_CRYPTO_SHA1_ARM_CE is not set
# CONFIG_CRYPTO_SHA2_ARM_CE is not set
# CONFIG_CRYPTO_SHA256_ARM is not set
# CONFIG_CRYPTO_SHA512_ARM is not set
CONFIG_CRYPTO_BLAKE2S_ARM=y
# CONFIG_CRYPTO_BLAKE2B_NEON is not set
CONFIG_CRYPTO_AES_ARM=y
CONFIG_CRYPTO_AES_ARM_BS=y
# CONFIG_CRYPTO_AES_ARM_CE is not set
# CONFIG_CRYPTO_GHASH_ARM_CE is not set
# CONFIG_CRYPTO_CRC32_ARM_CE is not set
CONFIG_CRYPTO_CHACHA20_NEON=y
CONFIG_CRYPTO_POLY1305_ARM=y
# CONFIG_CRYPTO_NHPOLY1305_NEON is not set
CONFIG_CRYPTO_CURVE25519_NEON=y
CONFIG_CRYPTO=y
CONFIG_CRYPTO_ALGAPI=y
CONFIG_CRYPTO_ALGAPI2=y
CONFIG_CRYPTO_AEAD=y
CONFIG_CRYPTO_AEAD2=y
CONFIG_CRYPTO_SKCIPHER=y
CONFIG_CRYPTO_SKCIPHER2=y
CONFIG_CRYPTO_HASH=y
CONFIG_CRYPTO_HASH2=y
CONFIG_CRYPTO_RNG=y
CONFIG_CRYPTO_RNG2=y
CONFIG_CRYPTO_AKCIPHER2=y
CONFIG_CRYPTO_AKCIPHER=y
CONFIG_CRYPTO_KPP2=y
CONFIG_CRYPTO_ACOMP2=y
CONFIG_CRYPTO_MANAGER=y
CONFIG_CRYPTO_MANAGER2=y
# CONFIG_CRYPTO_USER is not set
CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=y
CONFIG_CRYPTO_NULL=y
CONFIG_CRYPTO_NULL2=y
# CONFIG_CRYPTO_PCRYPT is not set
CONFIG_CRYPTO_CRYPTD=y
CONFIG_CRYPTO_AUTHENC=y
# CONFIG_CRYPTO_TEST is not set
CONFIG_CRYPTO_SIMD=y
CONFIG_CRYPTO_ENGINE=y
CONFIG_CRYPTO_RSA=y
# CONFIG_CRYPTO_DH is not set
# CONFIG_CRYPTO_ECDH is not set
# CONFIG_CRYPTO_ECDSA is not set
# CONFIG_CRYPTO_ECRDSA is not set
# CONFIG_CRYPTO_SM2 is not set
# CONFIG_CRYPTO_CURVE25519 is not set
# CONFIG_CRYPTO_CCM is not set
# CONFIG_CRYPTO_GCM is not set
# CONFIG_CRYPTO_CHACHA20POLY1305 is not set
# CONFIG_CRYPTO_AEGIS128 is not set
# CONFIG_CRYPTO_SEQIV is not set
# CONFIG_CRYPTO_ECHAINIV is not set
# CONFIG_CRYPTO_CBC is not set
# CONFIG_CRYPTO_CFB is not set
# CONFIG_CRYPTO_CTR is not set
# CONFIG_CRYPTO_CTS is not set
CONFIG_CRYPTO_ECB=y
# CONFIG_CRYPTO_LRW is not set
# CONFIG_CRYPTO_OFB is not set
# CONFIG_CRYPTO_PCBC is not set
CONFIG_CRYPTO_XTS=y
# CONFIG_CRYPTO_KEYWRAP is not set
# CONFIG_CRYPTO_ADIANTUM is not set
# CONFIG_CRYPTO_ESSIV is not set
# CONFIG_CRYPTO_CMAC is not set
# CONFIG_CRYPTO_HMAC is not set
# CONFIG_CRYPTO_XCBC is not set
# CONFIG_CRYPTO_VMAC is not set
CONFIG_CRYPTO_CRC32C=y
# CONFIG_CRYPTO_CRC32 is not set
# CONFIG_CRYPTO_XXHASH is not set
# CONFIG_CRYPTO_BLAKE2B is not set
# CONFIG_CRYPTO_BLAKE2S is not set
# CONFIG_CRYPTO_CRCT10DIF is not set
# CONFIG_CRYPTO_GHASH is not set
# CONFIG_CRYPTO_POLY1305 is not set
# CONFIG_CRYPTO_MD4 is not set
# CONFIG_CRYPTO_MD5 is not set
# CONFIG_CRYPTO_MICHAEL_MIC is not set
# CONFIG_CRYPTO_RMD160 is not set
# CONFIG_CRYPTO_SHA1 is not set
# CONFIG_CRYPTO_SHA256 is not set
# CONFIG_CRYPTO_SHA512 is not set
# CONFIG_CRYPTO_SHA3 is not set
# CONFIG_CRYPTO_SM3 is not set
# CONFIG_CRYPTO_STREEBOG is not set
# CONFIG_CRYPTO_WP512 is not set
CONFIG_CRYPTO_AES=y
# CONFIG_CRYPTO_AES_TI is not set
# CONFIG_CRYPTO_BLOWFISH is not set
# CONFIG_CRYPTO_CAMELLIA is not set
# CONFIG_CRYPTO_CAST5 is not set
# CONFIG_CRYPTO_CAST6 is not set
CONFIG_CRYPTO_DES=y
# CONFIG_CRYPTO_FCRYPT is not set
# CONFIG_CRYPTO_CHACHA20 is not set
# CONFIG_CRYPTO_SERPENT is not set
# CONFIG_CRYPTO_SM4 is not set
# CONFIG_CRYPTO_TWOFISH is not set
# CONFIG_CRYPTO_DEFLATE is not set
# CONFIG_CRYPTO_LZO is not set
# CONFIG_CRYPTO_842 is not set
# CONFIG_CRYPTO_LZ4 is not set
# CONFIG_CRYPTO_LZ4HC is not set
# CONFIG_CRYPTO_ZSTD is not set
# CONFIG_CRYPTO_ANSI_CPRNG is not set
# CONFIG_CRYPTO_DRBG_MENU is not set
# CONFIG_CRYPTO_JITTERENTROPY is not set
# CONFIG_CRYPTO_USER_API_HASH is not set
# CONFIG_CRYPTO_USER_API_SKCIPHER is not set
# CONFIG_CRYPTO_USER_API_RNG is not set
# CONFIG_CRYPTO_USER_API_AEAD is not set
CONFIG_CRYPTO_LIB_AES=y
CONFIG_CRYPTO_ARCH_HAVE_LIB_BLAKE2S=y
CONFIG_CRYPTO_LIB_BLAKE2S=y
CONFIG_CRYPTO_ARCH_HAVE_LIB_CHACHA=y
CONFIG_CRYPTO_LIB_CHACHA=y
CONFIG_CRYPTO_ARCH_HAVE_LIB_CURVE25519=y
CONFIG_CRYPTO_LIB_CURVE25519_GENERIC=y
CONFIG_CRYPTO_LIB_CURVE25519=y
CONFIG_CRYPTO_LIB_DES=y
CONFIG_CRYPTO_LIB_POLY1305_RSIZE=9
CONFIG_CRYPTO_ARCH_HAVE_LIB_POLY1305=y
CONFIG_CRYPTO_LIB_POLY1305=y
CONFIG_CRYPTO_LIB_CHACHA20POLY1305=y
CONFIG_CRYPTO_HW=y
CONFIG_CRYPTO_DEV_FSL_CAAM_COMMON=y
CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_DESC=y
CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API_DESC=y
CONFIG_CRYPTO_DEV_FSL_CAAM=y
# CONFIG_CRYPTO_DEV_FSL_CAAM_DEBUG is not set
CONFIG_CRYPTO_DEV_FSL_CAAM_JR=y
CONFIG_CRYPTO_DEV_FSL_CAAM_RINGSIZE=9
# CONFIG_CRYPTO_DEV_FSL_CAAM_INTC is not set
CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API=y
CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API=y
CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API=y
CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API=y
# CONFIG_CRYPTO_DEV_OMAP is not set
# CONFIG_CRYPTO_DEV_SAHARA is not set
# CONFIG_CRYPTO_DEV_ATMEL_AES is not set
# CONFIG_CRYPTO_DEV_ATMEL_TDES is not set
# CONFIG_CRYPTO_DEV_ATMEL_SHA is not set
# CONFIG_CRYPTO_DEV_ATMEL_ECC is not set
# CONFIG_CRYPTO_DEV_ATMEL_SHA204A is not set
# CONFIG_CRYPTO_DEV_MXS_DCP is not set
# CONFIG_CRYPTO_DEV_VIRTIO is not set
# CONFIG_CRYPTO_DEV_STM32_CRC is not set
# CONFIG_CRYPTO_DEV_STM32_HASH is not set
# CONFIG_CRYPTO_DEV_STM32_CRYP is not set
# CONFIG_CRYPTO_DEV_SAFEXCEL is not set
# CONFIG_CRYPTO_DEV_CCREE is not set
# CONFIG_CRYPTO_DEV_AMLOGIC_GXL is not set

I didn't try to understand and fix that. If you need more information,
just tell me.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |

Attachments:

(No filename) (30.42 kB)
signature.asc (488.00 B)
Download all attachments

2021-12-22 10:37:59

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

Hello again,

I mistyped an address in my report's Cc: list. If you respond please
s/xx/x/. My keyboard's x key sometimes gerates two x :-\

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |

Attachments:

(No filename) (313.00 B)
signature.asc (488.00 B)
Download all attachments

2021-12-29 02:06:02

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Wed, Dec 22, 2021 at 11:22:46AM +0100, Uwe Kleine-K?nig wrote:
>
> I still experience a problem with the patch that got
> adad556efcdd42a1d9e060cbe5f6161cccf1fa28 in v5.16-rc1. I saw there are
> two commit fixing this one (
>
> cad439fc040e crypto: api - Do not create test larvals if manager is disabled
> e42dff467ee6 crypto: api - Export crypto_boot_test_finished
>
> ) but I still encounter the following on 2f47a9a4dfa3:

Perhaps you missed the last fix?

commit beaaaa37c664e9afdf2913aee19185d8e3793b50
Author: Herbert Xu <[email protected]>
Date: Fri Nov 5 15:26:08 2021 +0800

crypto: api - Fix boot-up crash when crypto manager is disabled

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2021-12-29 11:05:39

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Wed, Dec 29, 2021 at 01:05:54PM +1100, Herbert Xu wrote:
> On Wed, Dec 22, 2021 at 11:22:46AM +0100, Uwe Kleine-K?nig wrote:
> >
> > I still experience a problem with the patch that got
> > adad556efcdd42a1d9e060cbe5f6161cccf1fa28 in v5.16-rc1. I saw there are
> > two commit fixing this one (
> >
> > cad439fc040e crypto: api - Do not create test larvals if manager is disabled
> > e42dff467ee6 crypto: api - Export crypto_boot_test_finished
> >
> > ) but I still encounter the following on 2f47a9a4dfa3:
>
> Perhaps you missed the last fix?
>
> commit beaaaa37c664e9afdf2913aee19185d8e3793b50
> Author: Herbert Xu <[email protected]>
> Date: Fri Nov 5 15:26:08 2021 +0800
>
> crypto: api - Fix boot-up crash when crypto manager is disabled

As 2f47a9a4dfa3 includes this commit, this is not the problem.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |

Attachments:

(No filename) (0.98 kB)
signature.asc (488.00 B)
Download all attachments

2022-03-17 05:19:55

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Wed, Dec 29, 2021 at 12:05:23PM +0100, Uwe Kleine-K?nig wrote:
> On Wed, Dec 29, 2021 at 01:05:54PM +1100, Herbert Xu wrote:
> > On Wed, Dec 22, 2021 at 11:22:46AM +0100, Uwe Kleine-K?nig wrote:
> > >
> > > I still experience a problem with the patch that got
> > > adad556efcdd42a1d9e060cbe5f6161cccf1fa28 in v5.16-rc1. I saw there are
> > > two commit fixing this one (
> > >
> > > cad439fc040e crypto: api - Do not create test larvals if manager is disabled
> > > e42dff467ee6 crypto: api - Export crypto_boot_test_finished
> > >
> > > ) but I still encounter the following on 2f47a9a4dfa3:
> >
> > Perhaps you missed the last fix?
> >
> > commit beaaaa37c664e9afdf2913aee19185d8e3793b50
> > Author: Herbert Xu <[email protected]>
> > Date: Fri Nov 5 15:26:08 2021 +0800
> >
> > crypto: api - Fix boot-up crash when crypto manager is disabled
>
> As 2f47a9a4dfa3 includes this commit, this is not the problem.

Using the config snippet in this email thread I was unable to
reproduce the failure under qemu. Can you still reproduce this
with the latest upstream kernel? If yes please send me your complete
config file.

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

2022-03-17 05:46:55

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Wed, Mar 16, 2022 at 05:37:19PM +0100, Uwe Kleine-K?nig wrote:
> On Wed, Mar 16, 2022 at 01:10:41PM +1200, Herbert Xu wrote:
> > On Wed, Dec 29, 2021 at 12:05:23PM +0100, Uwe Kleine-K?nig wrote:
> > > On Wed, Dec 29, 2021 at 01:05:54PM +1100, Herbert Xu wrote:
> > > > On Wed, Dec 22, 2021 at 11:22:46AM +0100, Uwe Kleine-K?nig wrote:
> > > > >
> > > > > I still experience a problem with the patch that got
> > > > > adad556efcdd42a1d9e060cbe5f6161cccf1fa28 in v5.16-rc1. I saw there are
> > > > > two commit fixing this one (
> > > > >
> > > > > cad439fc040e crypto: api - Do not create test larvals if manager is disabled
> > > > > e42dff467ee6 crypto: api - Export crypto_boot_test_finished
> > > > >
> > > > > ) but I still encounter the following on 2f47a9a4dfa3:
> > > >
> > > > Perhaps you missed the last fix?
> > > >
> > > > commit beaaaa37c664e9afdf2913aee19185d8e3793b50
> > > > Author: Herbert Xu <[email protected]>
> > > > Date: Fri Nov 5 15:26:08 2021 +0800
> > > >
> > > > crypto: api - Fix boot-up crash when crypto manager is disabled
> > >
> > > As 2f47a9a4dfa3 includes this commit, this is not the problem.
> >
> > Using the config snippet in this email thread I was unable to
> > reproduce the failure under qemu. Can you still reproduce this
> > with the latest upstream kernel? If yes please send me your complete
> > config file.
>
> Still happens on 5.17-rc8, config attached.

I debugged that a bit further because the problem is in the way while
debugging another bug. What I learned is that without
CONFIG_DEBUG_BUG_VERBOSE a BUG results in hitting an undefined
instruction e7f001f2 on ARM.

After enabling CONFIG_DEBUG_BUG_VERBOSE this gets much more helpful:

[ 1.630337] kernel BUG at crypto/algapi.c:461!

Digging a bit deeper the problem is that simd_skcipher_create_compat()
fails for aes_algs[1] in arch/arm/crypto/aes-neonbs-glue.c with -ENOENT
and then aes_exit -> simd_skcipher_free -> crypto_unregister_skcipher ->
crypto_unregister_alg stumbles over refcount_read(&alg->cra_refcnt)
being 2. Is this enough to understand the actual problem?

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |

Attachments:

(No filename) (2.29 kB)
signature.asc (499.00 B)
Download all attachments

2022-03-17 06:19:29

[permalink] [raw]

Subject: Re: [v2 PATCH] crypto: api - Fix built-in testing dependency failures

On Wed, Mar 16, 2022 at 01:10:41PM +1200, Herbert Xu wrote:
> On Wed, Dec 29, 2021 at 12:05:23PM +0100, Uwe Kleine-K?nig wrote:
> > On Wed, Dec 29, 2021 at 01:05:54PM +1100, Herbert Xu wrote:
> > > On Wed, Dec 22, 2021 at 11:22:46AM +0100, Uwe Kleine-K?nig wrote:
> > > >
> > > > I still experience a problem with the patch that got
> > > > adad556efcdd42a1d9e060cbe5f6161cccf1fa28 in v5.16-rc1. I saw there are
> > > > two commit fixing this one (
> > > >
> > > > cad439fc040e crypto: api - Do not create test larvals if manager is disabled
> > > > e42dff467ee6 crypto: api - Export crypto_boot_test_finished
> > > >
> > > > ) but I still encounter the following on 2f47a9a4dfa3:
> > >
> > > Perhaps you missed the last fix?
> > >
> > > commit beaaaa37c664e9afdf2913aee19185d8e3793b50
> > > Author: Herbert Xu <[email protected]>
> > > Date: Fri Nov 5 15:26:08 2021 +0800
> > >
> > > crypto: api - Fix boot-up crash when crypto manager is disabled
> >
> > As 2f47a9a4dfa3 includes this commit, this is not the problem.
>
> Using the config snippet in this email thread I was unable to
> reproduce the failure under qemu. Can you still reproduce this
> with the latest upstream kernel? If yes please send me your complete
> config file.

Still happens on 5.17-rc8, config attached.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |

Attachments:

(No filename) (0.00 B)
signature.asc (499.00 B)
Download all attachments

2022-03-17 06:42:18