Re: [PATCH] hwrng: cctrng: Add cancel_work_sync before module remove

[Herbert Xu]

Pei Xiao <xiaopei01@xxxxxxxxxx> wrote:
> Be ensured that the work is canceled before proceeding with
> the cleanup in cc_trng_pm_fini.
> 
> Fixes: a583ed310bb6 ("hwrng: cctrng - introduce Arm CryptoCell driver")
> Signed-off-by: Pei Xiao <xiaopei01@xxxxxxxxxx>
> ---
> drivers/char/hw_random/cctrng.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/char/hw_random/cctrng.c b/drivers/char/hw_random/cctrng.c
> index 4db198849695..fd1ee3687782 100644
> --- a/drivers/char/hw_random/cctrng.c
> +++ b/drivers/char/hw_random/cctrng.c
> @@ -127,6 +127,8 @@ static void cc_trng_pm_fini(struct cctrng_drvdata *drvdata)
> {
>        struct device *dev = &(drvdata->pdev->dev);
> 
> +       cancel_work_sync(&drvdata->compwork);
> +       cancel_work_sync(&drvdata->startwork);
>        pm_runtime_disable(dev);
> }

I don't think this is right.  This is a problem with using devres:
you need to be very careful with unwinding things.  The remove
occurs before all devm resources are released.  So the device is
still registered with the hwrng core and potentially live.

These work tasks are created by the ISR.  So they should only be
cancelled after the IRQ handler has been unregistered.

Perhaps we should just rip out all the devm code and go back to
manual freeing.

Cheers,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt