Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH -next] crypto: jitter - change module_init(jent_mod_init)
 to subsys_initcall(jent_mod_init)
To:     Herbert Xu <herbert@gondor.apana.org.au>, <cuigaosheng1@huawei.com>
CC:     <davem@davemloft.net>, <linux-crypto@vger.kernel.org>
References: <20230425125709.39470-1-cuigaosheng1@huawei.com>
 <ZEjkmOPvk7Iz213G@gondor.apana.org.au>
From:   cuigaosheng <cuigaosheng1@huawei.com>
Message-ID: <d3198a93-3811-69d3-9a19-602bf8b849aa@huawei.com>
Date:   Wed, 26 Apr 2023 17:18:11 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.1
MIME-Version: 1.0
In-Reply-To: <ZEjkmOPvk7Iz213G@gondor.apana.org.au>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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,