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