| Message ID | 20230314180733.4167836-1-meenakshi.aggarwal@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: caam/jr - fix algorithms exit/cleanup | expand |
On Tue, Mar 14, 2023 at 11:37:33PM +0530, meenakshi.aggarwal@nxp.com wrote: > From: Horia GeantA <horia.geanta@nxp.com> > > Current implementations is not being able to gracefully unregister > crypto algorithms from crypto API while there are users / allocated > tfms (cra_refcnt > 1). > > When unloading the caam_jr module, the .remove callback for the last JR > device exits with -EBUSY and doesn't perform the clean-up > (crypto algorithms unregistering etc.). > > One side effect of this is leading to an oops, which occurs due to a > corruption in the linked list of "misc devices" > (drivers/char/misc.c - misc_list) > > Signed-off-by: Horia GeantA <horia.geanta@nxp.com> > --- > drivers/crypto/caam/jr.c | 39 +++++++++++++++++++++++++++++++++------ > 1 file changed, 33 insertions(+), 6 deletions(-) Sorry, I don't think this is the right way to go. We need to disassociate the registration of the Crypto algorithms (soft) from the presence of the hardware (hard). Sure, you can certainly refuse to register any algorithms until the first piece of hardware shows up. But once you are registered, you must not unregister yourself from the Crypto API until the module actually goes away (when built-in that means never). The complexity then goes into making sure that your driver fails gracefully when the device goes away in the *middle* of a crypto computation. Cheers,
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index 724fdec18bf9..9918ae0a33a7 100644 --- a/drivers/crypto/caam/jr.c +++ b/drivers/crypto/caam/jr.c @@ -27,8 +27,36 @@ static struct jr_driver_data driver_data; static DEFINE_MUTEX(algs_lock); static unsigned int active_devs; -static void register_algs(struct caam_drv_private_jr *jrpriv, - struct device *dev) +static void init_misc_func(struct caam_drv_private_jr *jrpriv, + struct device *dev) +{ + mutex_lock(&algs_lock); + + if (active_devs != 1) + goto algs_unlock; + + jrpriv->hwrng = !caam_rng_init(dev); + +algs_unlock: + mutex_unlock(&algs_lock); +} + +static void exit_misc_func(struct caam_drv_private_jr *jrpriv, + struct device *dev) +{ + mutex_lock(&algs_lock); + + if (active_devs != 1) + goto algs_unlock; + + if (jrpriv->hwrng) + caam_rng_exit(dev); + +algs_unlock: + mutex_unlock(&algs_lock); +} + +static void register_algs(struct device *dev) { mutex_lock(&algs_lock); @@ -38,7 +66,6 @@ static void register_algs(struct caam_drv_private_jr *jrpriv, caam_algapi_init(dev); caam_algapi_hash_init(dev); caam_pkc_init(dev); - jrpriv->hwrng = !caam_rng_init(dev); caam_prng_register(dev); caam_qi_algapi_init(dev); @@ -136,8 +163,7 @@ static int caam_jr_remove(struct platform_device *pdev) jrdev = &pdev->dev; jrpriv = dev_get_drvdata(jrdev); - if (jrpriv->hwrng) - caam_rng_exit(jrdev->parent); + exit_misc_func(jrpriv, jrdev->parent); /* * Return EBUSY if job ring already allocated. @@ -595,7 +621,8 @@ static int caam_jr_probe(struct platform_device *pdev) atomic_set(&jrpriv->tfm_count, 0); - register_algs(jrpriv, jrdev->parent); + register_algs(jrdev->parent); + init_misc_func(jrpriv, jrdev->parent); return 0; }