Re: [PATCH] crypto: api - Allow delayed algorithm destruction

[Sean Anderson]

On 4/8/25 23:29, Herbert Xu wrote:
> On Wed, Apr 09, 2025 at 10:58:04AM +0800, Herbert Xu wrote:
>>
>> What I'll do is make the crypto_unregister call wait for the users
>> to go away.  That matches how the network device unregistration works
>> and hopefully should solve this problem.  But keep your eyes for
>> dead locks that used to plague netdev unregistration :)
> 
> That was a dumb idea.  All it would do is make the shutdown hang.
> So here is a different tack.  Let the algorithms stick around,
> by allocating them dynamically instead.  Then we could simply
> kfree them when the user finally disappears (if ever).
> 
> Note to make this work, caam needs to be modified to allocate the
> algorithms dynamically (kmemdup should work), and add a cra_destroy
> function to kfree the memory.
> 
> ---8<---
> The current algorithm unregistration mechanism originated from
> software crypto.  The code relies on module reference counts to
> stop in-use algorithms from being unregistered.  Therefore if
> the unregistration function is reached, it is assumed that the
> module reference count has hit zero and thus the algorithm reference
> count should be exactly 1.
> 
> This is completely broken for hardware devices, which can be
> unplugged at random.
> 
> Fix this by allowing algorithms to be destroyed later if a destroy
> callback is provided.
> 
> Reported-by: Sean Anderson <sean.anderson@xxxxxxxxx>
> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> 
> diff --git a/crypto/algapi.c b/crypto/algapi.c
> index 5b8a4c787387..f368c0dc0d6d 100644
> --- a/crypto/algapi.c
> +++ b/crypto/algapi.c
> @@ -481,10 +481,10 @@ void crypto_unregister_alg(struct crypto_alg *alg)
>  	if (WARN(ret, "Algorithm %s is not registered", alg->cra_driver_name))
>  		return;
>  
> -	if (WARN_ON(refcount_read(&alg->cra_refcnt) != 1))
> -		return;
> -
> -	if (alg->cra_type && alg->cra_type->destroy)
> +	if (alg->cra_destroy)
> +		crypto_alg_put(alg);
> +	else if (!WARN_ON(refcount_read(&alg->cra_refcnt) != 1) &&
> +		 alg->cra_type && alg->cra_type->destroy)
>  		alg->cra_type->destroy(alg);
>  
>  	crypto_remove_final(&list);

The above patch didn't apply cleanly. I seem to be missing cra_type. What
tree should I test with?

--Sean