[v2] power: supply: da9150: Fix use after free bug in da9150_charger_remove due to race condition

Message ID 20230311174650.3979517-1-zyytlz.wz@163.com
State New
Headers
Series [v2] power: supply: da9150: Fix use after free bug in da9150_charger_remove due to race condition |

Commit Message

Zheng Wang March 11, 2023, 5:46 p.m. UTC
  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 da9150_charger_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>
---
v2:
- fix wrong description in commit message and mov cancel_work_sync
after usb_unregister_notifier suggested by Sebastian Reichel
---
 drivers/power/supply/da9150-charger.c | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Sebastian Reichel March 12, 2023, 10:31 p.m. UTC | #1
Hi,

On Sun, Mar 12, 2023 at 01:46:50AM +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 da9150_charger_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>
> ---
> v2:
> - fix wrong description in commit message and mov cancel_work_sync
> after usb_unregister_notifier suggested by Sebastian Reichel
> ---

Thanks, queued to power-supply's fixes branch. Please make sure you
send your patches to the correct destination next time (linux-scsi
should be linux-pm).

-- 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..a87aeaea38e1 100644
> --- a/drivers/power/supply/da9150-charger.c
> +++ b/drivers/power/supply/da9150-charger.c
> @@ -657,6 +657,7 @@ static int da9150_charger_remove(struct platform_device *pdev)
>  
>  	if (!IS_ERR_OR_NULL(charger->usb_phy))
>  		usb_unregister_notifier(charger->usb_phy, &charger->otg_nb);
> +	cancel_work_sync(&charger->otg_work);
>  
>  	power_supply_unregister(charger->battery);
>  	power_supply_unregister(charger->usb);
> -- 
> 2.25.1
>
  

Patch

diff --git a/drivers/power/supply/da9150-charger.c b/drivers/power/supply/da9150-charger.c
index 14da5c595dd9..a87aeaea38e1 100644
--- a/drivers/power/supply/da9150-charger.c
+++ b/drivers/power/supply/da9150-charger.c
@@ -657,6 +657,7 @@  static int da9150_charger_remove(struct platform_device *pdev)
 
 	if (!IS_ERR_OR_NULL(charger->usb_phy))
 		usb_unregister_notifier(charger->usb_phy, &charger->otg_nb);
+	cancel_work_sync(&charger->otg_work);
 
 	power_supply_unregister(charger->battery);
 	power_supply_unregister(charger->usb);