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

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

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

cctrng schedules two deferred works that dereference the driver data via
container_of(): compwork (cc_trng_compwork_handler) is queued from the
interrupt handler cc_isr() on an RNG completion interrupt, and startwork
(cc_trng_startwork_handler) is queued from the hwrng read path.

The driver data is allocated with devm_kzalloc() and nothing ever cancels
these works. On unbind, cctrng_remove() only runs the PM teardown, and
devres then releases the hwrng registration, the IRQ and finally the
driver data. free_irq() stops further interrupts but does not cancel a
work the handler has already queued, so a completion interrupt arriving
shortly before removal can leave compwork pending and have
cc_trng_compwork_handler() run against the freed cctrng_drvdata
(use-after-free). The hwrng core does not drain this driver-private work.

Register the works with devm_work_autocancel() instead of INIT_WORK().
They are set up before devm_request_irq() and devm_hwrng_register(), so on
unwind devres unregisters the hwrng and frees the IRQ first - after which
neither work can be scheduled any more - and then cancels the works while
the driver data is still valid. The handlers do not re-queue themselves,
so a single cancel drains them.

Found by static analysis and code review; not runtime-reproduced (needs
Arm CryptoCell TRNG hardware and a completion interrupt racing unbind).

Fixes: a583ed310bb6 ("hwrng: cctrng - introduce Arm CryptoCell driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 drivers/char/hw_random/cctrng.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/cctrng.c b/drivers/char/hw_random/cctrng.c
index 03401ee..138cc70 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>
@@ -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 */
-- 
2.43.0