crypto: api - Redo lookup on EEXIST

On Mon, May 19, 2025 at 10:09:10AM +0200, Ingo Franzki wrote:
>
> During this weekend's CI run, we got the following:
> 
>     alg: aead: error allocating gcm_base(ctr(aes-generic),ghash-generic) (generic impl of gcm(aes)): -17
>     alg: self-tests for gcm(aes) using gcm-aes-s390 failed (rc=-17)
> 
> Last week, we had a similar failure:
> 
>     aes_s390: Allocating AES fallback algorithm ctr(aes) failed
>     alg: skcipher: failed to allocate transform for ctr-aes-s390: -17
>     alg: self-tests for ctr(aes) using ctr-aes-s390 failed (rc=-17)

Please try this patch:

---8<---
When two crypto algorithm lookups occur at the same time with
different names for the same algorithm, e.g., ctr(aes-generic)
and ctr(aes), they will both be instantiated.  However, only one
of them can be registered.  The second instantiation will fail
with EEXIST.

Avoid failing the second lookup by making it retry, but only once
because there are tricky names such as gcm_base(ctr(aes),ghash)
that will always fail, despite triggering instantiation and EEXIST.

Reported-by: Ingo Franzki <ifranzki@linux.ibm.com>
Fixes: 2825982d9d66 ("[CRYPTO] api: Added event notification")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

On 19.05.2025 15:47, Ingo Franzki wrote:
> On 19.05.2025 12:29, Herbert Xu wrote:
>> On Mon, May 19, 2025 at 10:09:10AM +0200, Ingo Franzki wrote:
>>>
>>> During this weekend's CI run, we got the following:
>>>
>>>     alg: aead: error allocating gcm_base(ctr(aes-generic),ghash-generic) (generic impl of gcm(aes)): -17
>>>     alg: self-tests for gcm(aes) using gcm-aes-s390 failed (rc=-17)
>>>
>>> Last week, we had a similar failure:
>>>
>>>     aes_s390: Allocating AES fallback algorithm ctr(aes) failed
>>>     alg: skcipher: failed to allocate transform for ctr-aes-s390: -17
>>>     alg: self-tests for ctr(aes) using ctr-aes-s390 failed (rc=-17)
>>
>> Please try this patch:
> 
> Since I can't reproduce the problem at will, I can't tell if your patch solves the problem. From what I can tell it looks reasonable. 
> Nevertheless, I have added your patch to be applied on top of the next day's next kernel tree in our CI so that it runs with your patch for a while.
> Lets run it for a few days and see if the error still shows up or not. 

It has now run several days in our CI with this patch on top of Next without showing the error again, so I would claim that your patch fixes the problem.
Can you please include it into the next kernel? 

> 
>>
>> ---8<---
>> When two crypto algorithm lookups occur at the same time with
>> different names for the same algorithm, e.g., ctr(aes-generic)
>> and ctr(aes), they will both be instantiated.  However, only one
>> of them can be registered.  The second instantiation will fail
>> with EEXIST.
>>
>> Avoid failing the second lookup by making it retry, but only once
>> because there are tricky names such as gcm_base(ctr(aes),ghash)
>> that will always fail, despite triggering instantiation and EEXIST.
>>
>> Reported-by: Ingo Franzki <ifranzki@linux.ibm.com>
>> Fixes: 2825982d9d66 ("[CRYPTO] api: Added event notification")
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>>
>> diff --git a/crypto/api.c b/crypto/api.c
>> index 133d9b626922..5724d62e9d07 100644
>> --- a/crypto/api.c
>> +++ b/crypto/api.c
>> @@ -219,10 +219,19 @@ static struct crypto_alg *crypto_larval_wait(struct crypto_alg *alg,
>>  		if (crypto_is_test_larval(larval))
>>  			crypto_larval_kill(larval);
>>  		alg = ERR_PTR(-ETIMEDOUT);
>> -	} else if (!alg) {
>> +	} else if (!alg || PTR_ERR(alg) == -EEXIST) {
>> +		int err = alg ? -EEXIST : -EAGAIN;
>> +
>> +		/*
>> +		 * EEXIST is expected because two probes can be scheduled
>> +		 * at the same time with one using alg_name and the other
>> +		 * using driver_name.  Do a re-lookup but do not retry in
>> +		 * case we hit a quirk like gcm_base(ctr(aes),...) which
>> +		 * will never match.
>> +		 */
>>  		alg = &larval->alg;
>>  		alg = crypto_alg_lookup(alg->cra_name, type, mask) ?:
>> -		      ERR_PTR(-EAGAIN);
>> +		      ERR_PTR(err);
>>  	} else if (IS_ERR(alg))
>>  		;
>>  	else if (crypto_is_test_larval(larval) &&
> 
>

On Mon, May 26, 2025 at 08:44:33AM +0200, Ingo Franzki wrote:
>
> It has now run several days in our CI with this patch on top of Next without showing the error again, so I would claim that your patch fixes the problem.
> Can you please include it into the next kernel? 

Great! The patch is already in my pull request for 6.16.

Thanks,