[PATCH v2] platform/x86: lenovo-yoga-tab2-pro-1380-fastcharger: cancel work on teardown

[hyunjungg]

From: Hyunjung Ko <hj351016@xxxxxxxxx>

fc->work is scheduled from the extcon notifier callback
yt2_1380_fc_extcon_evt() (and once from probe), but it is never cancelled.
The containing struct yt2_1380_fc and the extcon notifier are both
device-managed on the serdev device, and the serdev driver has no
->remove callback, so teardown happens entirely via devres unwinding
when yt2_1380_fc_pdev_remove() calls serdev_device_remove().

devm_extcon_register_notifier_all() unregistering on teardown only stops
future notifications; it does not drain a work item that the notifier has
already queued. A fast-charger extcon event that races device unbind can
therefore queue fc->work just before teardown; the work then runs after
devres has torn the device down. yt2_1380_fc_worker() dereferences not
only fc itself but also fc->pinctrl (pinctrl_select_state()) and the
serdev (serdev_device_write_buf()), so it must be drained before those
device-managed resources are released, not merely before fc is freed.

Cancel the work via devm_add_action_or_reset(), registered after the
serdev is opened and pinctrl is acquired but before
devm_extcon_register_notifier_all(). Because devres runs cleanup in
reverse order, teardown then unregisters the notifier first (so it can no
longer queue the work), next cancels the work while fc->pinctrl and the
serdev are still valid, and only then closes the serdev, releases pinctrl
and frees fc. This closes both the re-arm window and the use-after-free
of fc and of its sub-resources in the worker.

Found by static analysis / code review; compile-tested only, not
reproduced on hardware (requires the Lenovo Yoga Tablet 2 1380 charger).

Fixes: b2ed33e8d486 ("platform/x86: Add lenovo-yoga-tab2-pro-1380-fastcharger driver")
Signed-off-by: Hyunjung Ko <hj351016@xxxxxxxxx>
---
v2:
 - Move the devm_add_action_or_reset(cancel_work) registration from right
   after INIT_WORK to after devm_serdev_device_open()/devm_pinctrl_get()
   and before devm_extcon_register_notifier_all(). The v1 placement
   registered the cancel action first, so devres (LIFO) drained the work
   last - after the serdev was closed and pinctrl released - leaving the
   worker's fc->pinctrl / serdev dereferences racing freed resources.
   The new placement drains the work while those resources are still live.

 drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c b/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c
--- a/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c
+++ b/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c
@@ -148,6 +148,11 @@
 	.write_wakeup = serdev_device_write_wakeup,
 };

+static void yt2_1380_fc_cancel_work(void *data)
+{
+	cancel_work_sync(data);
+}
+
 static int yt2_1380_fc_serdev_probe(struct serdev_device *serdev)
 {
 	struct device *dev = &serdev->dev;
@@ -209,6 +214,18 @@
 	serdev_device_set_baudrate(serdev, 600);
 	serdev_device_set_flow_control(serdev, false);

+	/*
+	 * Register work cancellation after the serdev is opened and pinctrl
+	 * is acquired, but before the extcon notifier.  devres unwinds in
+	 * reverse, so teardown unregisters the notifier first (no new work
+	 * can be queued), then cancels the work while fc->pinctrl and the
+	 * serdev are still valid, and only then closes the serdev and
+	 * releases pinctrl.
+	 */
+	ret = devm_add_action_or_reset(dev, yt2_1380_fc_cancel_work, &fc->work);
+	if (ret)
+		return ret;
+
 	ret = devm_extcon_register_notifier_all(dev, fc->extcon, &fc->nb);
 	if (ret)
 		return dev_err_probe(dev, ret, "registering extcon notifier\n");