Re: [PATCH] power: supply: bq24190: stop IRQ and work before supply removal

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 2649 bytes --]

Hi,

On Wed, Nov 04, 2026 at 05:47:54PM +0800, Yaohan Li wrote:
> bq24190_remove() unregisters its power supplies while a managed IRQ
> handler and input_current_limit_work may still access them. The
> managed IRQ is only released after remove() returns, so stopping the
> supplies first leaves a window for a callback to use freed memory.
> 
> Release and synchronize the IRQ before unregistering the supplies,
> and cancel the delayed current-limit work. Cancel the work once more
> after unregistering to drain any instance requeued by an in-flight
> power-supply callback.
> 
> A KASAN test with a simulated I2C device and the in-tree charger
> driver reproduced the race during removal. The same test completed
> without the use-after-free after the change.
> 
> Fixes: 4f73a52df7d28278 ("power: supply: bq24190: Avoid rescheduling after cancelling work")
> Assisted-by: ZCode:GLM-4
> Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
> ---

The mistake is registering runtime PM and power_supply without
device managed resources. Instead of manually unrolling the device
amanged resources in the reomve function, fix the probe side
instead resulting in a nice cleanup as a side effect.

Greetings,

-- Sebastian

>  drivers/power/supply/bq24190_charger.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
> index 7a5867dd74c23..fd0ea5385f0dd 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -2265,9 +2265,24 @@ static void bq24190_remove(struct i2c_client *client)
>  		dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error);
>  
>  	bq24190_register_reset(bdi);
> +
> +	/*
> +	 * The IRQ handler and the input current limit work both access the
> +	 * power supplies. Stop them before the supplies are unregistered
> +	 * and freed below: the devm cleanup only runs after .remove() has
> +	 * returned. The second cancel catches work re-queued by an
> +	 * external_power_changed() callback which was still running while
> +	 * the supplies were unregistered (the work has a 300 ms delay).
> +	 */
> +	devm_free_irq(bdi->dev, client->irq, bdi);
> +	cancel_delayed_work_sync(&bdi->input_current_limit_work);
> +
>  	if (bdi->battery)
>  		power_supply_unregister(bdi->battery);
>  	power_supply_unregister(bdi->charger);
> +
> +	cancel_delayed_work_sync(&bdi->input_current_limit_work);
> +
>  	if (error >= 0)
>  		pm_runtime_put_sync(bdi->dev);
>  	pm_runtime_dont_use_autosuspend(bdi->dev);
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]