[PATCH] power: supply: bq25890: cancel pump work before supply removal

[PATCH] power: supply: bq25890: cancel pump work before supply removal

[Yaohan Li]

The existing devm cleanup for pump_express_work runs after the
managed power supply has been unregistered. A delayed or running pump
work item can therefore call power_supply_changed() with a freed
power_supply pointer.

Register an additional devm cleanup action after power-supply
registration, so devres reverse ordering drains the work while the
supply is still alive. Keep the original late cleanup as a backstop
for work that might be queued again during supplier teardown.

A KASAN test using a simulated I2C device and the real bq25890
driver reproduced the use-after-free before the change and not under
the corresponding teardown sequence afterward.

Fixes: a7aaa80098d5b7608 ("power: supply: bq25890: Ensure pump_express_work is cancelled on remove")
Assisted-by: ZCode:GLM-4
Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
---
 drivers/power/supply/bq25890_charger.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index f7510ada6b734..a3e35e036b97b 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -1459,6 +1459,23 @@ static void bq25890_non_devm_cleanup(void *data)
 	}
 }
 
+static void bq25890_cancel_pump_express(void *data)
+{
+	struct bq25890_device *bq = data;
+
+	/*
+	 * Cancel the delayed work while the power supply is still alive:
+	 * bq25890_pump_express_work() calls power_supply_changed() at its
+	 * end and must not run against an already released supply.
+	 *
+	 * bq25890_non_devm_cleanup() stays in place as well: it runs
+	 * after the devres-managed power supply has been unregistered,
+	 * catching work queued again from the supplier-change path
+	 * between this cancel and the unregister.
+	 */
+	cancel_delayed_work_sync(&bq->pump_express_work);
+}
+
 static int bq25890_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -1531,6 +1548,10 @@ static int bq25890_probe(struct i2c_client *client)
 	if (ret < 0)
 		return dev_err_probe(dev, ret, "registering power supply\n");
 
+	ret = devm_add_action_or_reset(dev, bq25890_cancel_pump_express, bq);
+	if (ret)
+		return ret;
+
 	ret = devm_request_threaded_irq(dev, client->irq, NULL,
 					bq25890_irq_handler_thread,
 					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-- 
2.34.1

Re: [PATCH] power: supply: bq25890: cancel pump work before supply removal

[Sebastian Reichel]

Hi,

On Wed, Nov 04, 2026 at 05:47:31PM +0800, Yaohan Li wrote:
> The existing devm cleanup for pump_express_work runs after the
> managed power supply has been unregistered. A delayed or running pump
> work item can therefore call power_supply_changed() with a freed
> power_supply pointer.
> 
> Register an additional devm cleanup action after power-supply
> registration, so devres reverse ordering drains the work while the
> supply is still alive. Keep the original late cleanup as a backstop
> for work that might be queued again during supplier teardown.
> 
> A KASAN test using a simulated I2C device and the real bq25890
> driver reproduced the use-after-free before the change and not under
> the corresponding teardown sequence afterward.
> 
> Fixes: a7aaa80098d5b7608 ("power: supply: bq25890: Ensure pump_express_work is cancelled on remove")
> Assisted-by: ZCode:GLM-4
> Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
> ---
>  drivers/power/supply/bq25890_charger.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index f7510ada6b734..a3e35e036b97b 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -1459,6 +1459,23 @@ static void bq25890_non_devm_cleanup(void *data)
>  	}
>  }
>  
> +static void bq25890_cancel_pump_express(void *data)
> +{
> +	struct bq25890_device *bq = data;
> +
> +	/*
> +	 * Cancel the delayed work while the power supply is still alive:
> +	 * bq25890_pump_express_work() calls power_supply_changed() at its
> +	 * end and must not run against an already released supply.
> +	 *
> +	 * bq25890_non_devm_cleanup() stays in place as well: it runs
> +	 * after the devres-managed power supply has been unregistered,
> +	 * catching work queued again from the supplier-change path
> +	 * between this cancel and the unregister.
> +	 */
> +	cancel_delayed_work_sync(&bq->pump_express_work);
> +}
> +
>  static int bq25890_probe(struct i2c_client *client)
>  {
>  	struct device *dev = &client->dev;
> @@ -1531,6 +1548,10 @@ static int bq25890_probe(struct i2c_client *client)
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "registering power supply\n");
>  
> +	ret = devm_add_action_or_reset(dev, bq25890_cancel_pump_express, bq);
> +	if (ret)
> +		return ret;

Use devm_delayed_work_autocancel() (and drop the INIT_DELAYED_WORK).

-- Sebastian

> +
>  	ret = devm_request_threaded_irq(dev, client->irq, NULL,
>  					bq25890_irq_handler_thread,
>  					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -- 
> 2.34.1
>