Re: [PATCH 08/11] power: supply: pm8916_bms_vm: Fix use-after-free in power_supply_changed()

[Nikita Travkin <nikita@trvn.ru>]

Waqar Hameed писал(а) 21.12.2025 03:36:
> Using the `devm_` variant for requesting IRQ _before_ the `devm_`
> variant for allocating/registering the `power_supply` handle, means that
> the `power_supply` handle will be deallocated/unregistered _before_ the
> interrupt handler (since `devm_` naturally deallocates in reverse
> allocation order). This means that during removal, there is a race
> condition where an interrupt can fire just _after_ the `power_supply`
> handle has been freed, *but* just _before_ the corresponding
> unregistration of the IRQ handler has run.
> 
> This will lead to the IRQ handler calling `power_supply_changed()` with
> a freed `power_supply` handle. Which usually crashes the system or
> otherwise silently corrupts the memory...
> 
> Note that there is a similar situation which can also happen during
> `probe()`; the possibility of an interrupt firing _before_ registering
> the `power_supply` handle. This would then lead to the nasty situation
> of using the `power_supply` handle *uninitialized* in
> `power_supply_changed()`.
> 
> Fix this racy use-after-free by making sure the IRQ is requested _after_
> the registration of the `power_supply` handle.
> 
> Fixes: 098bce1838e0 ("power: supply: Add pm8916 VM-BMS support")
> Signed-off-by: Waqar Hameed <waqar.hameed@axis.com>

Reviewed-by: Nikita Travkin <nikita@trvn.ru>

Thanks for fixing this!
Nikita

> ---
>  drivers/power/supply/pm8916_bms_vm.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/power/supply/pm8916_bms_vm.c b/drivers/power/supply/pm8916_bms_vm.c
> index 5120be086e6ff..de5d571c03e21 100644
> --- a/drivers/power/supply/pm8916_bms_vm.c
> +++ b/drivers/power/supply/pm8916_bms_vm.c
> @@ -167,15 +167,6 @@ static int pm8916_bms_vm_battery_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return -EINVAL;
>  
> -	irq = platform_get_irq_byname(pdev, "fifo");
> -	if (irq < 0)
> -		return irq;
> -
> -	ret = devm_request_threaded_irq(dev, irq, NULL, pm8916_bms_vm_fifo_update_done_irq,
> -					IRQF_ONESHOT, "pm8916_vm_bms", bat);
> -	if (ret)
> -		return ret;
> -
>  	ret = regmap_bulk_read(bat->regmap, bat->reg + PM8916_PERPH_TYPE, &tmp, 2);
>  	if (ret)
>  		goto comm_error;
> @@ -220,6 +211,15 @@ static int pm8916_bms_vm_battery_probe(struct platform_device *pdev)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Unable to get battery info\n");
>  
> +	irq = platform_get_irq_byname(pdev, "fifo");
> +	if (irq < 0)
> +		return irq;
> +
> +	ret = devm_request_threaded_irq(dev, irq, NULL, pm8916_bms_vm_fifo_update_done_irq,
> +					IRQF_ONESHOT, "pm8916_vm_bms", bat);
> +	if (ret)
> +		return ret;
> +
>  	platform_set_drvdata(pdev, bat);
>  
>  	return 0;