From: Horia GeantA <[email protected]>
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 <[email protected]>
---
drivers/crypto/caam/jr.c | 39 +++++++++++++++++++++++++++++++++------
1 file changed, 33 insertions(+), 6 deletions(-)
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;
}
--
2.25.1
On Tue, Mar 14, 2023 at 11:37:33PM +0530, [email protected] wrote:
> From: Horia GeantA <[email protected]>
>
> 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 <[email protected]>
> ---
> 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,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt