[PATCH] hwrng: cctrng: Fix use-after-free in cctrng_remove due to race condition

Pei Xiao posted 1 patch 1 month, 4 weeks ago
drivers/char/hw_random/cctrng.c | 6 ++++++
1 file changed, 6 insertions(+)
[PATCH] hwrng: cctrng: Fix use-after-free in cctrng_remove due to race condition
Posted by Pei Xiao 1 month, 4 weeks ago
In cctrng_probe, &drvdata->compwork is bound with
cc_trng_compwork_handler, and &drvdata->startwork is bound with
cc_trng_startwork_handler. cc_isr can schedule compwork on system_wq
when an RNG interrupt is received, and cctrng_read can schedule
startwork on system_wq when the data buffer needs refilling.

If we remove the device, cctrng_remove makes cleanup and the memory
allocated for drvdata with devm_kzalloc() is released by the devm
cleanup after the remove callback returns, while the works mentioned
above may still be pending or running. The sequence of operations that
may lead to a UAF bug is as follows:

CPU0                                      CPU1

                                          | cc_isr
                                          | schedule_work(&drvdata->compwork)
cctrng_remove                             |
cc_trng_pm_fini(drvdata)                  |
// remove returns                         |
// devm cleanup: free_irq,                |
// kfree(drvdata)                         |
                                          | cc_trng_compwork_handler
                                          | // use drvdata (use-after-free)

Fix it by masking the RNG interrupts, so the IRQ handler cannot
schedule new work, and canceling the works before the remaining
cleanup in cctrng_remove and the devm release of drvdata.

Fixes: a583ed310bb6 ("hwrng: cctrng - introduce Arm CryptoCell driver")
Assisted-by: Codex:deepseek-v4-flash
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/char/hw_random/cctrng.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/char/hw_random/cctrng.c b/drivers/char/hw_random/cctrng.c
index a6925211c3b5..dd41f2d1fee5 100644
--- a/drivers/char/hw_random/cctrng.c
+++ b/drivers/char/hw_random/cctrng.c
@@ -568,6 +568,12 @@ static void cctrng_remove(struct platform_device *pdev)
 
 	cc_trng_pm_fini(drvdata);
 
+	/* Mask RNG interrupts so cc_isr cannot schedule new work */
+	cc_iowrite(drvdata, CC_RNG_IMR_REG_OFFSET, 0xFFFFFFFF);
+
+	cancel_work_sync(&drvdata->compwork);
+	cancel_work_sync(&drvdata->startwork);
+
 	dev_info(dev, "ARM cctrng device terminated\n");
 }
 
-- 
2.25.1
Re: [PATCH] hwrng: cctrng: Fix use-after-free in cctrng_remove due to race condition
Posted by Herbert Xu 1 month, 2 weeks ago
On Tue, Aug 04, 2026 at 03:34:41PM +0800, Pei Xiao wrote:
>
> diff --git a/drivers/char/hw_random/cctrng.c b/drivers/char/hw_random/cctrng.c
> index a6925211c3b5..dd41f2d1fee5 100644
> --- a/drivers/char/hw_random/cctrng.c
> +++ b/drivers/char/hw_random/cctrng.c
> @@ -568,6 +568,12 @@ static void cctrng_remove(struct platform_device *pdev)
>  
>  	cc_trng_pm_fini(drvdata);
>  
> +	/* Mask RNG interrupts so cc_isr cannot schedule new work */
> +	cc_iowrite(drvdata, CC_RNG_IMR_REG_OFFSET, 0xFFFFFFFF);
> +
> +	cancel_work_sync(&drvdata->compwork);
> +	cancel_work_sync(&drvdata->startwork);

I don't think this closes the race window.  After all, the ISR could
have already been started before your iowrite call.

Is there any way to make devm make the cancel_work_sync calls after
the ISR has been deregistered?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] hwrng: cctrng: Fix use-after-free in cctrng_remove due to race condition
Posted by Pei Xiao 1 month, 2 weeks ago

在 2026/8/15 08:57, Herbert Xu 写道:
> On Tue, Aug 04, 2026 at 03:34:41PM +0800, Pei Xiao wrote:
>>
>> diff --git a/drivers/char/hw_random/cctrng.c b/drivers/char/hw_random/cctrng.c
>> index a6925211c3b5..dd41f2d1fee5 100644
>> --- a/drivers/char/hw_random/cctrng.c
>> +++ b/drivers/char/hw_random/cctrng.c
>> @@ -568,6 +568,12 @@ static void cctrng_remove(struct platform_device *pdev)
>>  
>>  	cc_trng_pm_fini(drvdata);
>>  
>> +	/* Mask RNG interrupts so cc_isr cannot schedule new work */
>> +	cc_iowrite(drvdata, CC_RNG_IMR_REG_OFFSET, 0xFFFFFFFF);
>> +
>> +	cancel_work_sync(&drvdata->compwork);
>> +	cancel_work_sync(&drvdata->startwork);
> 
> I don't think this closes the race window.  After all, the ISR could
> have already been started before your iowrite call.
> 
> Is there any way to make devm make the cancel_work_sync calls after
> the ISR has been deregistered?
Hi Maintainer,
  Thanks your reply.
  How about this:
diff --git a/drivers/char/hw_random/cctrng.c
b/drivers/char/hw_random/cctrng.c
index a6925211c3b5..c1536660e8d5 100644
--- a/drivers/char/hw_random/cctrng.c
+++ b/drivers/char/hw_random/cctrng.c
@@ -11,6 +11,7 @@
 #include <linux/interrupt.h>
 #include <linux/irqreturn.h>
 #include <linux/workqueue.h>
+#include <linux/devm-helpers.h>
 #include <linux/circ_buf.h>
 #include <linux/completion.h>
 #include <linux/of.h>
@@ -502,8 +503,16 @@ 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 */

Thanks!

> 
> Thanks,