Skip to content

Commit 8d59cf3

Browse files
whamesre
authored andcommitted
power: supply: sbs-battery: Fix use-after-free in power_supply_changed()
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: d2cec82 ("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> Link: https://patch.msgid.link/0ef896e002495e615157b482d18a437af19ddcd0.1766268280.git.waqar.hameed@axis.com Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
1 parent e2febe3 commit 8d59cf3

1 file changed

Lines changed: 18 additions & 18 deletions

File tree

drivers/power/supply/sbs-battery.c

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,24 +1174,6 @@ static int sbs_probe(struct i2c_client *client)
11741174

11751175
i2c_set_clientdata(client, chip);
11761176

1177-
if (!chip->gpio_detect)
1178-
goto skip_gpio;
1179-
1180-
irq = gpiod_to_irq(chip->gpio_detect);
1181-
if (irq <= 0) {
1182-
dev_warn(&client->dev, "Failed to get gpio as irq: %d\n", irq);
1183-
goto skip_gpio;
1184-
}
1185-
1186-
rc = devm_request_threaded_irq(&client->dev, irq, NULL, sbs_irq,
1187-
IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
1188-
dev_name(&client->dev), chip);
1189-
if (rc) {
1190-
dev_warn(&client->dev, "Failed to request irq: %d\n", rc);
1191-
goto skip_gpio;
1192-
}
1193-
1194-
skip_gpio:
11951177
/*
11961178
* Before we register, we might need to make sure we can actually talk
11971179
* to the battery.
@@ -1217,6 +1199,24 @@ static int sbs_probe(struct i2c_client *client)
12171199
return dev_err_probe(&client->dev, PTR_ERR(chip->power_supply),
12181200
"Failed to register power supply\n");
12191201

1202+
if (!chip->gpio_detect)
1203+
goto out;
1204+
1205+
irq = gpiod_to_irq(chip->gpio_detect);
1206+
if (irq <= 0) {
1207+
dev_warn(&client->dev, "Failed to get gpio as irq: %d\n", irq);
1208+
goto out;
1209+
}
1210+
1211+
rc = devm_request_threaded_irq(&client->dev, irq, NULL, sbs_irq,
1212+
IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
1213+
dev_name(&client->dev), chip);
1214+
if (rc) {
1215+
dev_warn(&client->dev, "Failed to request irq: %d\n", rc);
1216+
goto out;
1217+
}
1218+
1219+
out:
12201220
dev_info(&client->dev,
12211221
"%s: battery gas gauge device registered\n", client->name);
12221222

0 commit comments

Comments
 (0)