| Message ID | 20230425125709.39470-1-cuigaosheng1@huawei.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [-next] crypto: jitter - change module_init(jent_mod_init) to subsys_initcall(jent_mod_init) | expand |
On Tue, Apr 25, 2023 at 08:57:09PM +0800, Gaosheng Cui wrote: > The ecdh-nist-p256 algorithm will depend on jitterentropy_rng, > and when they are built into kernel, the order of registration > should be done such that the underlying algorithms are ready > before the ones on top are registered. > > Now ecdh is initialized with subsys_initcall but jitterentropy_rng > is initialized with module_init. > > This patch will change module_init(jent_mod_init) to > subsys_initcall(jent_mod_init), so jitterentropy_rng will be > registered before ecdh-nist-p256 when they are built into kernel. > > Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> > --- > crypto/jitterentropy-kcapi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c > index b9edfaa51b27..563c1ea8c8fe 100644 > --- a/crypto/jitterentropy-kcapi.c > +++ b/crypto/jitterentropy-kcapi.c > @@ -205,7 +205,7 @@ static void __exit jent_mod_exit(void) > crypto_unregister_rng(&jent_alg); > } > > -module_init(jent_mod_init); > +subsys_initcall(jent_mod_init); This is unnecessary because we now postpone the testing of all algorithms until their first use. So unless something in the kernel makes use of this before/during module_init, then we don't need to move jent. Cheers,
Thanks for taking time to review this patch.
We have not used subsystem initialisation ordering to guarantee the
order of registration since commit adad556efcdd ("crypto: api - Fix
built-in testing dependency failures"),but this patch is not a bugfix,
it's not merged into the earlier stable branch.
For example,on linux-5.10-y, when they are built into the kernel,
ecdh will test failed on earlier kernel, we can enable fips=1,
the calltrace like below:
alg: ecdh: test failed on vector 2, err=-14
Kernel panic - not syncing: alg: self-tests for ecdh-generic (ecdh)
failed in fips mode!
Call Trace:
dump_stack+0x57/0x6e
panic+0x109/0x2ca
alg_test+0x414/0x420
? __switch_to_asm+0x3a/0x60
? __switch_to_asm+0x34/0x60
? __schedule+0x263/0x640
? crypto_acomp_scomp_free_ctx+0x30/0x30
cryptomgr_test+0x22/0x40
kthread+0xf9/0x130
? kthread_park+0x90/0x90
ret_from_fork+0x22/0x30
Merging the commit adad556efcdd ("crypto: api - Fix built-in testing dependency failures")
into linux-5.10-y can fix it, but we have to merge more patch before it,
such as: https://www.spinics.net/lists/linux-crypto/msg57562.html
Otherwise this patch will introduce new problems.
It might make sense to keep the timing, on the other hand, we can use
it to fix the dependency problem of earlier kernel.
Thanks.
Gaosheng.
On 2023/4/26 16:45, Herbert Xu wrote:
> On Tue, Apr 25, 2023 at 08:57:09PM +0800, Gaosheng Cui wrote:
>> The ecdh-nist-p256 algorithm will depend on jitterentropy_rng,
>> and when they are built into kernel, the order of registration
>> should be done such that the underlying algorithms are ready
>> before the ones on top are registered.
>>
>> Now ecdh is initialized with subsys_initcall but jitterentropy_rng
>> is initialized with module_init.
>>
>> This patch will change module_init(jent_mod_init) to
>> subsys_initcall(jent_mod_init), so jitterentropy_rng will be
>> registered before ecdh-nist-p256 when they are built into kernel.
>>
>> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
>> ---
>> crypto/jitterentropy-kcapi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
>> index b9edfaa51b27..563c1ea8c8fe 100644
>> --- a/crypto/jitterentropy-kcapi.c
>> +++ b/crypto/jitterentropy-kcapi.c
>> @@ -205,7 +205,7 @@ static void __exit jent_mod_exit(void)
>> crypto_unregister_rng(&jent_alg);
>> }
>>
>> -module_init(jent_mod_init);
>> +subsys_initcall(jent_mod_init);
> This is unnecessary because we now postpone the testing of all
> algorithms until their first use. So unless something in the
> kernel makes use of this before/during module_init, then we don't
> need to move jent.
>
> Cheers,
On Wed, Apr 26, 2023 at 05:18:11PM +0800, cuigaosheng wrote: > Thanks for taking time to review this patch. > > We have not used subsystem initialisation ordering to guarantee the > order of registration since commit adad556efcdd ("crypto: api - Fix > built-in testing dependency failures"),but this patch is not a bugfix, > it's not merged into the earlier stable branch. You're going about this backwards. We don't apply patches to the mainline kernel to fix problems that only exist in an older version. If you have a problem with an older kernel then you should fix it there. Cheers,
Submitting patches directly to stable branches will not be accepted. Maybe it doesn't need fixing，we can also compile ecdh into modules to get around this problem. Thanks for your time. On 2023/4/26 17:26, Herbert Xu wrote: > On Wed, Apr 26, 2023 at 05:18:11PM +0800, cuigaosheng wrote: >> Thanks for taking time to review this patch. >> >> We have not used subsystem initialisation ordering to guarantee the >> order of registration since commit adad556efcdd ("crypto: api - Fix >> built-in testing dependency failures"),but this patch is not a bugfix, >> it's not merged into the earlier stable branch. > You're going about this backwards. We don't apply patches to > the mainline kernel to fix problems that only exist in an older > version. > > If you have a problem with an older kernel then you should fix > it there. > > Cheers,
diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c index b9edfaa51b27..563c1ea8c8fe 100644 --- a/crypto/jitterentropy-kcapi.c +++ b/crypto/jitterentropy-kcapi.c @@ -205,7 +205,7 @@ static void __exit jent_mod_exit(void) crypto_unregister_rng(&jent_alg); } -module_init(jent_mod_init); +subsys_initcall(jent_mod_init); module_exit(jent_mod_exit); MODULE_LICENSE("Dual BSD/GPL");
The ecdh-nist-p256 algorithm will depend on jitterentropy_rng, and when they are built into kernel, the order of registration should be done such that the underlying algorithms are ready before the ones on top are registered. Now ecdh is initialized with subsys_initcall but jitterentropy_rng is initialized with module_init. This patch will change module_init(jent_mod_init) to subsys_initcall(jent_mod_init), so jitterentropy_rng will be registered before ecdh-nist-p256 when they are built into kernel. Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> --- crypto/jitterentropy-kcapi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)