| Message ID | 20230309181036.262674-1-zyytlz.wz@163.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | power: supply: da9150: Fix use after free bug in da9150_charger_remove due to race condition | expand |
Hi, On Fri, Mar 10, 2023 at 02:10:36AM +0800, Zheng Wang wrote: > In da9150_charger_probe, &charger->otg_work is bound with > da9150_charger_otg_work. da9150_charger_otg_ncb may be > called to start the work. > > If we remove the module which will call da9150_charger_remove > to make cleanup, there may be a unfinished work. The possible > sequence is as follows: > > Fix it by canceling the work before cleanup in the mtk_jpeg_remove mtk_jpeg_remove? > > CPU0 CPUc1 > > |da9150_charger_otg_work > da9150_charger_remove | > power_supply_unregister | > device_unregister | > power_supply_dev_release| > kfree(psy) | > | > | power_supply_changed(charger->usb); > | //use > Fixes: c1a281e34dae ("power: Add support for DA9150 Charger") > Signed-off-by: Zheng Wang <zyytlz.wz@163.com> > --- Looks correct to me, but the cancel_work_sync should happen after usb_unregister_notifier(), otherwise there is still a race condition. -- Sebastian > drivers/power/supply/da9150-charger.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/power/supply/da9150-charger.c b/drivers/power/supply/da9150-charger.c > index 14da5c595dd9..41b68f2f6ed8 100644 > --- a/drivers/power/supply/da9150-charger.c > +++ b/drivers/power/supply/da9150-charger.c > @@ -642,6 +642,7 @@ static int da9150_charger_remove(struct platform_device *pdev) > struct da9150_charger *charger = platform_get_drvdata(pdev); > int irq; > > + cancel_work_sync(&charger->otg_work); > /* Make sure IRQs are released before unregistering power supplies */ > irq = platform_get_irq_byname(pdev, "CHG_VBUS"); > free_irq(irq, charger); > -- > 2.25.1 >
diff --git a/drivers/power/supply/da9150-charger.c b/drivers/power/supply/da9150-charger.c index 14da5c595dd9..41b68f2f6ed8 100644 --- a/drivers/power/supply/da9150-charger.c +++ b/drivers/power/supply/da9150-charger.c @@ -642,6 +642,7 @@ static int da9150_charger_remove(struct platform_device *pdev) struct da9150_charger *charger = platform_get_drvdata(pdev); int irq; + cancel_work_sync(&charger->otg_work); /* Make sure IRQs are released before unregistering power supplies */ irq = platform_get_irq_byname(pdev, "CHG_VBUS"); free_irq(irq, charger);
In da9150_charger_probe, &charger->otg_work is bound with da9150_charger_otg_work. da9150_charger_otg_ncb may be called to start the work. If we remove the module which will call da9150_charger_remove to make cleanup, there may be a unfinished work. The possible sequence is as follows: Fix it by canceling the work before cleanup in the mtk_jpeg_remove CPU0 CPUc1 |da9150_charger_otg_work da9150_charger_remove | power_supply_unregister | device_unregister | power_supply_dev_release| kfree(psy) | | | power_supply_changed(charger->usb); | //use Fixes: c1a281e34dae ("power: Add support for DA9150 Charger") Signed-off-by: Zheng Wang <zyytlz.wz@163.com> --- drivers/power/supply/da9150-charger.c | 1 + 1 file changed, 1 insertion(+)