[PATCH v2] Input: wm831x-ts - drain pen-down work at teardown

Fan Wu posted 1 patch 2 days, 18 hours ago
drivers/input/touchscreen/wm831x-ts.c | 10 ++++++++++
1 file changed, 10 insertions(+)
[PATCH v2] Input: wm831x-ts - drain pen-down work at teardown
Posted by Fan Wu 2 days, 18 hours ago
The pen-down and data IRQ handlers queue pd_data_work to switch between the
two IRQs.  The worker obtains wm831x_ts with container_of() and calls
enable_irq().  free_irq() synchronizes an IRQ handler, but does not drain a
work item already queued by that handler.

wm831x_ts_remove() can therefore free the IRQ actions while pd_data_work is
pending or running.  The work may then dereference wm831x_ts after devres
frees it (use-after-free), or re-enable an IRQ whose action has been freed.

The input device is devm-allocated, so the input core unregisters it after
wm831x_ts_remove() returns.  If a userspace handle is still open, that
unregister drives the input close callback, which on pen-down calls
enable_irq(pd_irq) - after wm831x_ts_remove() has already freed the pd_irq
action.  Unregister the input device before freeing the IRQs, so the close
callback runs while the actions are still installed.  This must precede the
IRQ teardown: disabling and cancelling first would still let the close
callback re-enable pd_irq after its action is gone.

After unregistering, drain the work: disable both IRQs so neither handler
can queue another instance, cancel_work_sync() to make the drain final,
then free the actions.  Apply the same drain to err_pd_irq; the input
device is not registered there, so it is not unregistered.

This issue was found by an in-house static analysis tool and confirmed by
manual code review.

Fixes: f5346668150c ("Input: wm831x-ts - fix races with IRQ management")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
Changes since v1:
- Unregister the input device before freeing the IRQs. v1 drained the
  work in wm831x_ts_remove(), but the input device is devm-managed and is
  unregistered by the input core only after remove() returns; with a
  userspace handle open, that unregister drives the close callback, which
  on pen-down calls enable_irq(pd_irq) after its action has been freed.
 drivers/input/touchscreen/wm831x-ts.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/input/touchscreen/wm831x-ts.c b/drivers/input/touchscreen/wm831x-ts.c
index 98f8ec408cad..a465b1b96fd8 100644
--- a/drivers/input/touchscreen/wm831x-ts.c
+++ b/drivers/input/touchscreen/wm831x-ts.c
@@ -366,6 +366,10 @@ static int wm831x_ts_probe(struct platform_device *pdev)
 	return 0;
 
 err_pd_irq:
+	disable_irq(wm831x_ts->pd_irq);
+	disable_irq(wm831x_ts->data_irq);
+	cancel_work_sync(&wm831x_ts->pd_data_work);
+
 	free_irq(wm831x_ts->pd_irq, wm831x_ts);
 err_data_irq:
 	free_irq(wm831x_ts->data_irq, wm831x_ts);
@@ -378,6 +382,12 @@ static void wm831x_ts_remove(struct platform_device *pdev)
 {
 	struct wm831x_ts *wm831x_ts = platform_get_drvdata(pdev);
 
+	input_unregister_device(wm831x_ts->input_dev);
+
+	disable_irq(wm831x_ts->pd_irq);
+	disable_irq(wm831x_ts->data_irq);
+	cancel_work_sync(&wm831x_ts->pd_data_work);
+
 	free_irq(wm831x_ts->pd_irq, wm831x_ts);
 	free_irq(wm831x_ts->data_irq, wm831x_ts);
 }
-- 
2.34.1