On 21/12/2025 06:36, Waqar Hameed wrote:
> 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. Keep the old behavior of
> just printing a warning in case of any failures during the IRQ request
> and finishing the probe successfully.
>
> Fixes: d2cec82c2880 ("power: sbs-battery: Request threaded irq and fix dev callback cookie")
> Signed-off-by: Waqar Hameed <waqar.hameed@axis.com>
Reviewed-by: Phil Reid <preid@electromag.com.au>
> ---
> drivers/power/supply/sbs-battery.c | 36 +++++++++++++++---------------
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
> index 943c82ee978f4..43c48196c1674 100644
> --- a/drivers/power/supply/sbs-battery.c
> +++ b/drivers/power/supply/sbs-battery.c
> @@ -1174,24 +1174,6 @@ static int sbs_probe(struct i2c_client *client)
>
> i2c_set_clientdata(client, chip);
>
> - if (!chip->gpio_detect)
> - goto skip_gpio;
> -
> - irq = gpiod_to_irq(chip->gpio_detect);
> - if (irq <= 0) {
> - dev_warn(&client->dev, "Failed to get gpio as irq: %d\n", irq);
> - goto skip_gpio;
> - }
> -
> - rc = devm_request_threaded_irq(&client->dev, irq, NULL, sbs_irq,
> - IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> - dev_name(&client->dev), chip);
> - if (rc) {
> - dev_warn(&client->dev, "Failed to request irq: %d\n", rc);
> - goto skip_gpio;
> - }
> -
> -skip_gpio:
> /*
> * Before we register, we might need to make sure we can actually talk
> * to the battery.
> @@ -1217,6 +1199,24 @@ static int sbs_probe(struct i2c_client *client)
> return dev_err_probe(&client->dev, PTR_ERR(chip->power_supply),
> "Failed to register power supply\n");
>
> + if (!chip->gpio_detect)
> + goto out;
> +
> + irq = gpiod_to_irq(chip->gpio_detect);
> + if (irq <= 0) {
> + dev_warn(&client->dev, "Failed to get gpio as irq: %d\n", irq);
> + goto out;
> + }
> +
> + rc = devm_request_threaded_irq(&client->dev, irq, NULL, sbs_irq,
> + IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> + dev_name(&client->dev), chip);
> + if (rc) {
> + dev_warn(&client->dev, "Failed to request irq: %d\n", rc);
> + goto out;
> + }
> +
> +out:
> dev_info(&client->dev,
> "%s: battery gas gauge device registered\n", client->name);
>
--
Regards
Phil Reid
ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au
23 Junction Parade, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid@electromag.com.au