[PATCH 1/5] extcon: intel-int3496: drain ID work before its GPIOs and extcon device are freed

[hyunjungg]

From: Hyunjung Ko <hj351016@xxxxxxxxx>

int3496_probe() registers the debounce work for device-managed
cancellation before it acquires any of the resources that the work
handler dereferences:

	devm_delayed_work_autocancel(dev, &data->work, int3496_do_usb_id);
	...
	data->gpio_usb_id  = devm_gpiod_get(dev, "id", ...);
	data->gpio_vbus_en = devm_gpiod_get(dev, "vbus", ...);
	data->gpio_usb_mux = devm_gpiod_get(dev, "mux", ...);
	data->edev = devm_extcon_dev_allocate(dev, int3496_cable);
	devm_extcon_dev_register(dev, data->edev);
	devm_request_threaded_irq(dev, data->usb_id_irq, ...);

devres releases resources in reverse registration order, so on unbind
the extcon device and all three GPIO descriptors are released first and
the work is cancelled last:

	/* free_irq() */
	devm_extcon_dev_unregister() / extcon freed
	gpiod_put(mux) / gpiod_put(vbus_en) / gpiod_put(usb_id)
	...
	cancel_delayed_work_sync(&data->work);		/* too late */

int3496_thread_isr() arms the work with mod_delayed_work() on every ID
pin edge, so a work instance can be pending or running at unbind. The
handler int3496_do_usb_id() dereferences data->gpio_usb_id
(gpiod_get_value_cansleep), data->gpio_usb_mux, data->gpio_vbus_en and
data->edev (extcon_set_state_sync) with no guard. If it runs in the
window after the GPIOs/extcon device have been released but before the
work is cancelled, it operates on freed resources -> use-after-free.

Register the work for cancellation after the GPIOs and extcon device
are acquired and registered (and before the IRQ, which arms it), so
devres (LIFO) frees the IRQ first, then cancels the work, and only then
releases the GPIOs and extcon device. devm_delayed_work_autocancel()
still performs INIT_DELAYED_WORK() before the work is first queued
(queue_delayed_work() below, and the IRQ handler, both run after it).

Found by static analysis / code review; compile-tested only, not
reproduced on hardware (requires an INT3496 ACPI USB-ID mux device).

Fixes: f94a5becabf4 ("extconn: Clean-up few drivers by using managed work init")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Hyunjung Ko <hj351016@xxxxxxxxx>
---
diff --git a/drivers/extcon/extcon-intel-int3496.c b/drivers/extcon/extcon-intel-int3496.c
index 7d16d5b..40774fb 100644
--- a/drivers/extcon/extcon-intel-int3496.c
+++ b/drivers/extcon/extcon-intel-int3496.c
@@ -130,9 +130,6 @@ static int int3496_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	data->dev = dev;
-	ret = devm_delayed_work_autocancel(dev, &data->work, int3496_do_usb_id);
-	if (ret)
-		return ret;
 
 	data->gpio_usb_id =
 		devm_gpiod_get(dev, "id", GPIOD_IN | GPIOD_FLAGS_BIT_NONEXCLUSIVE);
@@ -169,6 +166,10 @@ static int int3496_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	ret = devm_delayed_work_autocancel(dev, &data->work, int3496_do_usb_id);
+	if (ret)
+		return ret;
+
 	ret = devm_request_threaded_irq(dev, data->usb_id_irq,
 					NULL, int3496_thread_isr,
 					IRQF_SHARED | IRQF_ONESHOT |