[PATCH 3/5] extcon: max8997: drain MUIC IRQ work before extcon device is freed

[hyunjungg]

From: Hyunjung Ko <hj351016@xxxxxxxxx>

max8997_muic_probe() registers the MUIC interrupt work for device-managed
cancellation before it allocates and registers the extcon device that the
work handler dereferences:

	INIT_WORK(&info->irq_work, max8997_muic_irq_work);
	devm_work_autocancel(&pdev->dev, &info->irq_work, ...);
	...
	devm_request_threaded_irq(&pdev->dev, virq, ...);	/* arms work */
	...
	info->edev = devm_extcon_dev_allocate(&pdev->dev, ...);
	devm_extcon_dev_register(&pdev->dev, info->edev);

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_work_sync(&info->irq_work);		/* too late */

max8997_muic_irq_handler() queues irq_work with schedule_work() on every
MUIC interrupt, so a work instance can be pending or running at unbind.
The handler max8997_muic_irq_work() dereferences info->edev via
extcon_set_state_sync(). Its only early-return guard tests info->edev, but
info is itself devm-allocated and freed last, so in the teardown window
info->edev is a dangling (non-NULL) pointer rather than NULL and the guard
does not fire. If the work runs after the extcon device has been freed but
before the work is cancelled, it dereferences freed memory, i.e. a
use-after-free.

Allocate and register the extcon device before registering the work for
cancellation (the redundant INIT_WORK() is moved up with it), so devres
(LIFO) frees the IRQs first, then cancels the work, and only then frees
the extcon device. devm_work_autocancel() still performs INIT_WORK()
before the work is first queued (the IRQ handler, requested below, is the
only queuing site).

Found by static analysis / code review; compile-tested only, not
reproduced on hardware (requires a MAX8997 MUIC).

Fixes: 87ee8de23c9d ("extcon: extcon-max8997: Simplify driver using devm")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Hyunjung Ko <hj351016@xxxxxxxxx>
---
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 9cddf08..a54e7c4 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -650,6 +650,19 @@ static int max8997_muic_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, info);
 	mutex_init(&info->mutex);
 
+	/* External connector */
+	info->edev = devm_extcon_dev_allocate(&pdev->dev, max8997_extcon_cable);
+	if (IS_ERR(info->edev)) {
+		dev_err(&pdev->dev, "failed to allocate memory for extcon\n");
+		return PTR_ERR(info->edev);
+	}
+
+	ret = devm_extcon_dev_register(&pdev->dev, info->edev);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register extcon device\n");
+		return ret;
+	}
+
 	INIT_WORK(&info->irq_work, max8997_muic_irq_work);
 	ret = devm_work_autocancel(&pdev->dev, &info->irq_work,
 				   max8997_muic_irq_work);
@@ -678,19 +691,6 @@ static int max8997_muic_probe(struct platform_device *pdev)
 		}
 	}
 
-	/* External connector */
-	info->edev = devm_extcon_dev_allocate(&pdev->dev, max8997_extcon_cable);
-	if (IS_ERR(info->edev)) {
-		dev_err(&pdev->dev, "failed to allocate memory for extcon\n");
-		return PTR_ERR(info->edev);
-	}
-
-	ret = devm_extcon_dev_register(&pdev->dev, info->edev);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to register extcon device\n");
-		return ret;
-	}
-
 	if (pdata && pdata->muic_pdata) {
 		struct max8997_muic_platform_data *muic_pdata
 			= pdata->muic_pdata;