[PATCH] mfd: si476x-i2c: cancel RDS drainer work before freeing RDS FIFO

[PATCH] mfd: si476x-i2c: cancel RDS drainer work before freeing RDS FIFO

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

The RDS FIFO drainer work (core->rds_fifo_drainer) is scheduled from the
interrupt dispatcher si476x_core_get_and_signal_status() whenever an
SI476X_FM_RDS_INT interrupt is seen:

	if (status & SI476X_FM_RDS_INT)
		si476x_core_start_rds_drainer_once(core);

si476x_core_start_rds_drainer_once() runs the work on the shared system
workqueue via schedule_work(&core->rds_fifo_drainer). The work handler
si476x_core_drain_rds_fifo() dereferences @core and reads from
core->rds_fifo.

si476x_core_remove() does not cancel this work. It disables the interrupt
(or, in polling mode, cancels only the sibling core->status_monitor
delayed work) and then frees the RDS FIFO:

	if (client->irq)
		disable_irq(client->irq);
	else
		cancel_delayed_work_sync(&core->status_monitor);

	kfifo_free(&core->rds_fifo);

disable_irq() waits for the interrupt handler to finish but does not flush
or cancel work that the handler has already queued. A drainer work item
that was scheduled just before (or is still running at) remove time can
therefore run after kfifo_free() and after the devm-allocated @core itself
is freed, dereferencing freed memory and operating on a freed kfifo
(use-after-free). The sibling status_monitor work is drained on teardown
but this one is not.

Drain the RDS drainer work before freeing the FIFO (and before the
devm-managed @core is released) to close the window.

Found by code review; not runtime-reproduced (requires si476x AM/FM radio
hardware and an RDS interrupt racing device removal).

Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 drivers/mfd/si476x-i2c.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/si476x-i2c.c b/drivers/mfd/si476x-i2c.c
index 0000000..1111111 100644
--- a/drivers/mfd/si476x-i2c.c
+++ b/drivers/mfd/si476x-i2c.c
@@ -831,6 +831,8 @@ static void si476x_core_remove(struct i2c_client *client)
 	if (client->irq)
 		disable_irq(client->irq);
 	else
 		cancel_delayed_work_sync(&core->status_monitor);

+	cancel_work_sync(&core->rds_fifo_drainer);
+
 	kfifo_free(&core->rds_fifo);
 }