[PATCH 2/5] extcon: palmas: drain ID-detect work before the extcon device is freed

[hyunjungg]

From: Hyunjung Ko <hj351016@xxxxxxxxx>

palmas_usb_probe() registers the GPIO ID-detect work for device-managed
cancellation before it allocates and registers the extcon device that
the work handler dereferences:

	devm_delayed_work_autocancel(&pdev->dev, &palmas_usb->wq_detectid,
				     palmas_gpio_id_detect);
	...
	palmas_usb->edev = devm_extcon_dev_allocate(&pdev->dev, ...);
	devm_extcon_dev_register(&pdev->dev, palmas_usb->edev);
	...
	devm_request_threaded_irq(..., palmas_gpio_id_irq_handler, ...);

devres releases resources in reverse registration order, so on unbind
the extcon device is torn down first and the work is cancelled last:

	/* free_irq() */
	devm_extcon_dev_unregister() / extcon freed
	...
	cancel_delayed_work_sync(&palmas_usb->wq_detectid);	/* too late */

palmas_gpio_id_irq_handler() queues wq_detectid with a debounce delay on
every ID GPIO edge, so a work instance can be pending at unbind. The
handler palmas_gpio_id_detect() dereferences palmas_usb->edev via
extcon_set_state_sync(). Its only early-return guard tests
palmas_usb->id_gpiod (acquired before the work registration, so still
valid), not ->edev, so it does not prevent the access. If the work runs
in the window after the extcon device has been freed but before the
work is cancelled, it dereferences freed memory -> use-after-free.

Register the work for cancellation after the extcon device is allocated
and registered (and before the IRQs, which queue it), so devres (LIFO)
frees the IRQs first, then cancels the work, and only then frees the
extcon device. The ID GPIO the handler also uses is acquired earlier and
thus still outlives the work. devm_delayed_work_autocancel() still runs
INIT_DELAYED_WORK() before the work is first queued (the IRQ handler and
the explicit palmas_gpio_id_detect() call both run after it).

Found by static analysis / code review; compile-tested only, not
reproduced on hardware (requires a Palmas/TWL6035 USB PHY).

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-palmas.c b/drivers/extcon/extcon-palmas.c
index d339b86..f7496e5 100644
--- a/drivers/extcon/extcon-palmas.c
+++ b/drivers/extcon/extcon-palmas.c
@@ -237,12 +237,6 @@ static int palmas_usb_probe(struct platform_device *pdev)
 			palmas_usb->sw_debounce_jiffies = msecs_to_jiffies(debounce);
 	}
 
-	status = devm_delayed_work_autocancel(&pdev->dev,
-					      &palmas_usb->wq_detectid,
-					      palmas_gpio_id_detect);
-	if (status)
-		return status;
-
 	palmas->usb = palmas_usb;
 	palmas_usb->palmas = palmas;
 
@@ -265,6 +259,12 @@ static int palmas_usb_probe(struct platform_device *pdev)
 		return status;
 	}
 
+	status = devm_delayed_work_autocancel(&pdev->dev,
+					      &palmas_usb->wq_detectid,
+					      palmas_gpio_id_detect);
+	if (status)
+		return status;
+
 	if (palmas_usb->enable_id_detection) {
 		palmas_usb->id_otg_irq = regmap_irq_get_virq(palmas->irq_data,
 							     PALMAS_ID_OTG_IRQ);