Re: [PATCH] hwrng: cctrng - cancel deferred works on unbind to avoid use-after-free

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] hwrng: cctrng - runtime resume before enabling runtime PM in
  cctrng_probe() [1]
--

commit 8e1ce791cb06b2432af8b8abe4b88073b7383901
Author: Hyunjung Ko <hj351016@gmail.com>

hwrng: cctrng - cancel deferred works on unbind to avoid use-after-free

This commit replaces INIT_WORK() with devm_work_autocancel() for
compwork and startwork in cctrng_probe() so that pending work items are
canceled on driver unbind before drvdata is freed.

> diff --git a/drivers/char/hw_random/cctrng.c b/drivers/char/hw_random/cctrng.c
> index 03401ee59a8e8..138cc7072b023 100644
> --- a/drivers/char/hw_random/cctrng.c
> +++ b/drivers/char/hw_random/cctrng.c

[ ... ]

> @@ -497,8 +498,14 @@ static int cctrng_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(drvdata->clk),
>  				     "Failed to get or enable the clock\n");
>  
> -	INIT_WORK(&drvdata->compwork, cc_trng_compwork_handler);
> -	INIT_WORK(&drvdata->startwork, cc_trng_startwork_handler);
> +	rc = devm_work_autocancel(dev, &drvdata->compwork,
> +				  cc_trng_compwork_handler);
> +	if (rc)
> +		return rc;
> +	rc = devm_work_autocancel(dev, &drvdata->startwork,
> +				  cc_trng_startwork_handler);
> +	if (rc)
> +		return rc;
>  	spin_lock_init(&drvdata->read_lock);
>  
>  	/* register the driver isr function */

[Severity: High]
This problem wasn't introduced by this patch, but later in cctrng_probe(),
does cc_trng_pm_get() fail with -EACCES when CONFIG_PM is enabled [1]?

drivers/char/hw_random/cctrng.c:cctrng_probe() {
	...
	/* init PM */
	rc = cc_trng_pm_init(drvdata);
	if (rc)
		return dev_err_probe(dev, rc, "cc_trng_pm_init failed\n");

	/* increment device's usage counter */
	rc = cc_trng_pm_get(dev);
	if (rc)
		return dev_err_probe(dev, rc, "cc_trng_pm_get returned %x\n", rc);
	...
	/* All set, we can allow auto-suspend */
	cc_trng_pm_go(drvdata);
	...
}

cc_trng_pm_init() calls pm_runtime_set_active(), while pm_runtime_enable()
is deferred until cc_trng_pm_go() at the end of probe.  When
cc_trng_pm_get() calls pm_runtime_resume_and_get(), runtime PM is still
disabled and dev->power.last_status is RPM_INVALID, so rpm_resume()
returns -EACCES and cctrng_probe() aborts.

[1] https://sashiko.dev/#/bug/linux-6da7f352-ac7d-4b81-8569-a1008cfc3400

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107100238.3542588-1-hj351016@gmail.com?part=1