| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230918-power-uaf-v1-1-73c397178c42@axis.com>
Date: Mon, 18 Sep 2023 09:33:26 +0200
From: Vincent Whitchurch <vincent.whitchurch@...s.com>
To: Sebastian Reichel <sre@...nel.org>
CC: Linus Walleij <linus.walleij@...aro.org>,
Matti Vaittinen <mazziesaccount@...il.com>,
<linux-pm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<kernel@...s.com>, Vincent Whitchurch <vincent.whitchurch@...s.com>
Subject: [PATCH] power: supply: Fix info use-after-free
power_supply_uevent() which is called to emit a udev event on device
deletion attempts to use the info structure which is device-managed and
has been freed before this point. The use-after-free is triggered since
commit 699fb50d99039 ("drivers: base: Free devm resources when
unregistering a device").
Fix this by associating the devm resource with the parent device
instead, similar to recent fixes done in the input subsystem, such as
commit dd613a4e45f8 ("HID: uclogic: Correct devm device reference for
hidinput input_dev").
Note that the power supply subsystem allows drivers to register a device
without a parent (with a warning), in this case there is still a risk of
use-after-free since we have no other device to attach the devm to.
==================================================================
BUG: KASAN: slab-use-after-free in power_supply_battery_info_has_prop (power_supply_core.c:872)
Read of size 4 at addr 0000000062e59028 by task python3/27
Call Trace:
power_supply_battery_info_has_prop (power_supply_core.c:872)
power_supply_uevent (power_supply_sysfs.c:504)
dev_uevent (drivers/base/core.c:2590)
kobject_uevent_env (lib/kobject_uevent.c:558)
kobject_uevent (lib/kobject_uevent.c:643)
device_del (drivers/base/core.c:3266 drivers/base/core.c:3831)
device_unregister (drivers/base/core.c:3730 drivers/base/core.c:3854)
power_supply_unregister (power_supply_core.c:1608)
devm_power_supply_release (power_supply_core.c:1515)
release_nodes (drivers/base/devres.c:506)
devres_release_group (drivers/base/devres.c:669)
i2c_device_remove (drivers/i2c/i2c-core-base.c:629)
device_remove (drivers/base/dd.c:570)
device_release_driver_internal (drivers/base/dd.c:1274 drivers/base/dd.c:1295)
device_driver_detach (drivers/base/dd.c:1332)
unbind_store (drivers/base/bus.c:247)
...
Allocated by task 27:
devm_kmalloc (drivers/base/devres.c:119 drivers/base/devres.c:829)
power_supply_get_battery_info (include/linux/device.h:316 power_supply_core.c:626)
__power_supply_register (power_supply_core.c:1408)
devm_power_supply_register (power_supply_core.c:1544)
bq256xx_probe (bq256xx_charger.c:1539 bq256xx_charger.c:1727) bq256xx_charger
i2c_device_probe (drivers/i2c/i2c-core-base.c:584)
really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658)
__driver_probe_device (drivers/base/dd.c:800)
device_driver_attach (drivers/base/dd.c:1128)
bind_store (drivers/base/bus.c:273)
...
Freed by task 27:
kfree (mm/slab_common.c:1073)
release_nodes (drivers/base/devres.c:503)
devres_release_all (drivers/base/devres.c:536)
device_del (drivers/base/core.c:3829)
device_unregister (drivers/base/core.c:3730 drivers/base/core.c:3854)
power_supply_unregister (power_supply_core.c:1608)
devm_power_supply_release (power_supply_core.c:1515)
release_nodes (drivers/base/devres.c:506)
devres_release_group (drivers/base/devres.c:669)
i2c_device_remove (drivers/i2c/i2c-core-base.c:629)
device_remove (drivers/base/dd.c:570)
device_release_driver_internal (drivers/base/dd.c:1274 drivers/base/dd.c:1295)
device_driver_detach (drivers/base/dd.c:1332)
unbind_store (drivers/base/bus.c:247)
...
==================================================================
Fixes: 27a2195efa8d ("power: supply: core: auto-exposure of simple-battery data")
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@...s.com>
---
drivers/power/supply/power_supply_core.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 0b69fb7bafd8..2863b0a4dfc7 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -573,6 +573,7 @@ EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
int power_supply_get_battery_info(struct power_supply *psy,
struct power_supply_battery_info **info_out)
{
+ struct device *allocdev = psy->dev.parent ?: &psy->dev;
struct power_supply_resistance_temp_table *resist_table;
struct power_supply_battery_info *info;
struct device_node *battery_np = NULL;
@@ -623,7 +624,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
goto out_put_node;
}
- info = devm_kzalloc(&psy->dev, sizeof(*info), GFP_KERNEL);
+ info = devm_kzalloc(allocdev, sizeof(*info), GFP_KERNEL);
if (!info) {
err = -ENOMEM;
goto out_put_node;
@@ -776,7 +777,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
info->ocv_table_size[index] = tab_len;
table = info->ocv_table[index] =
- devm_kcalloc(&psy->dev, tab_len, sizeof(*table), GFP_KERNEL);
+ devm_kcalloc(allocdev, tab_len, sizeof(*table), GFP_KERNEL);
if (!info->ocv_table[index]) {
power_supply_put_battery_info(psy, info);
err = -ENOMEM;
@@ -796,7 +797,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
goto out_ret_pointer;
info->resist_table_size = len / (2 * sizeof(__be32));
- resist_table = info->resist_table = devm_kcalloc(&psy->dev,
+ resist_table = info->resist_table = devm_kcalloc(allocdev,
info->resist_table_size,
sizeof(*resist_table),
GFP_KERNEL);
@@ -825,17 +826,18 @@ EXPORT_SYMBOL_GPL(power_supply_get_battery_info);
void power_supply_put_battery_info(struct power_supply *psy,
struct power_supply_battery_info *info)
{
+ struct device *allocdev = psy->dev.parent ?: &psy->dev;
int i;
for (i = 0; i < POWER_SUPPLY_OCV_TEMP_MAX; i++) {
if (info->ocv_table[i])
- devm_kfree(&psy->dev, info->ocv_table[i]);
+ devm_kfree(allocdev, info->ocv_table[i]);
}
if (info->resist_table)
- devm_kfree(&psy->dev, info->resist_table);
+ devm_kfree(allocdev, info->resist_table);
- devm_kfree(&psy->dev, info);
+ devm_kfree(allocdev, info);
}
EXPORT_SYMBOL_GPL(power_supply_put_battery_info);
---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230918-power-uaf-6f7f1b585ec5
Best regards,
--
Vincent Whitchurch <vincent.whitchurch@...s.com>
Powered by blists - more mailing lists