Skip to content

Commit cd7459e

Browse files
rafaeljwopsiff
authored andcommitted
ACPI: battery: Add synchronization between interface updates
[ Upstream commit 399dbca ] There is no synchronization between different code paths in the ACPI battery driver that update its sysfs interface or its power supply class device interface. In some cases this results to functional failures due to race conditions. One example of this is when two ACPI notifications: - ACPI_BATTERY_NOTIFY_STATUS (0x80) - ACPI_BATTERY_NOTIFY_INFO (0x81) are triggered (by the platform firmware) in a row with a little delay in between after removing and reinserting a laptop battery. Both notifications cause acpi_battery_update() to be called and if the delay between them is sufficiently small, sysfs_add_battery() can be re-entered before battery->bat is set which leads to a duplicate sysfs entry error: sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 Workqueue: kacpi_notify acpi_os_execute_deferred Call Trace: <TASK> dump_stack_lvl+0x5d/0x80 sysfs_warn_dup.cold+0x17/0x23 sysfs_create_dir_ns+0xce/0xe0 kobject_add_internal+0xba/0x250 kobject_add+0x96/0xc0 ? get_device_parent+0xde/0x1e0 device_add+0xe2/0x870 __power_supply_register.part.0+0x20f/0x3f0 ? wake_up_q+0x4e/0x90 sysfs_add_battery+0xa4/0x1d0 [battery] acpi_battery_update+0x19e/0x290 [battery] acpi_battery_notify+0x50/0x120 [battery] acpi_ev_notify_dispatch+0x49/0x70 acpi_os_execute_deferred+0x1a/0x30 process_one_work+0x177/0x330 worker_thread+0x251/0x390 ? __pfx_worker_thread+0x10/0x10 kthread+0xd2/0x100 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x34/0x50 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1a/0x30 </TASK> kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory. There are also other scenarios in which analogous issues may occur. Address this by using a common lock in all of the code paths leading to updates of driver interfaces: ACPI Notify () handler, system resume callback and post-resume notification, device addition and removal. This new lock replaces sysfs_lock that has been used only in sysfs_remove_battery() which now is going to be always called under the new lock, so it doesn't need any internal locking any more. Fixes: 1066625 ("ACPI: battery: Install Notify() handler directly") Closes: https://lore.kernel.org/linux-acpi/20250910142653.313360-1-luogf2025@163.com/ Reported-by: GuangFei Luo <luogf2025@163.com> Tested-by: GuangFei Luo <luogf2025@163.com> Cc: 6.6+ <stable@vger.kernel.org> # 6.6+ Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit 1ed161347ad9dc0b40d69dbd528e71c4c941ae30)
1 parent 7dad1f7 commit cd7459e

File tree

1 file changed

+29
-14
lines changed

1 file changed

+29
-14
lines changed

drivers/acpi/battery.c

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ enum {
9494

9595
struct acpi_battery {
9696
struct mutex lock;
97-
struct mutex sysfs_lock;
97+
struct mutex update_lock;
9898
struct power_supply *bat;
9999
struct power_supply_desc bat_desc;
100100
struct acpi_device *device;
@@ -888,15 +888,12 @@ static int sysfs_add_battery(struct acpi_battery *battery)
888888

889889
static void sysfs_remove_battery(struct acpi_battery *battery)
890890
{
891-
mutex_lock(&battery->sysfs_lock);
892-
if (!battery->bat) {
893-
mutex_unlock(&battery->sysfs_lock);
891+
if (!battery->bat)
894892
return;
895-
}
893+
896894
battery_hook_remove_battery(battery);
897895
power_supply_unregister(battery->bat);
898896
battery->bat = NULL;
899-
mutex_unlock(&battery->sysfs_lock);
900897
}
901898

902899
static void find_battery(const struct dmi_header *dm, void *private)
@@ -1056,6 +1053,9 @@ static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
10561053

10571054
if (!battery)
10581055
return;
1056+
1057+
guard(mutex)(&battery->update_lock);
1058+
10591059
old = battery->bat;
10601060
/*
10611061
* On Acer Aspire V5-573G notifications are sometimes triggered too
@@ -1078,21 +1078,22 @@ static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
10781078
}
10791079

10801080
static int battery_notify(struct notifier_block *nb,
1081-
unsigned long mode, void *_unused)
1081+
unsigned long mode, void *_unused)
10821082
{
10831083
struct acpi_battery *battery = container_of(nb, struct acpi_battery,
10841084
pm_nb);
1085-
int result;
10861085

1087-
switch (mode) {
1088-
case PM_POST_HIBERNATION:
1089-
case PM_POST_SUSPEND:
1086+
if (mode == PM_POST_SUSPEND || mode == PM_POST_HIBERNATION) {
1087+
guard(mutex)(&battery->update_lock);
1088+
10901089
if (!acpi_battery_present(battery))
10911090
return 0;
10921091

10931092
if (battery->bat) {
10941093
acpi_battery_refresh(battery);
10951094
} else {
1095+
int result;
1096+
10961097
result = acpi_battery_get_info(battery);
10971098
if (result)
10981099
return result;
@@ -1104,7 +1105,6 @@ static int battery_notify(struct notifier_block *nb,
11041105

11051106
acpi_battery_init_alarm(battery);
11061107
acpi_battery_get_state(battery);
1107-
break;
11081108
}
11091109

11101110
return 0;
@@ -1182,6 +1182,8 @@ static int acpi_battery_update_retry(struct acpi_battery *battery)
11821182
{
11831183
int retry, ret;
11841184

1185+
guard(mutex)(&battery->update_lock);
1186+
11851187
for (retry = 5; retry; retry--) {
11861188
ret = acpi_battery_update(battery, false);
11871189
if (!ret)
@@ -1192,6 +1194,13 @@ static int acpi_battery_update_retry(struct acpi_battery *battery)
11921194
return ret;
11931195
}
11941196

1197+
static void sysfs_battery_cleanup(struct acpi_battery *battery)
1198+
{
1199+
guard(mutex)(&battery->update_lock);
1200+
1201+
sysfs_remove_battery(battery);
1202+
}
1203+
11951204
static int acpi_battery_add(struct acpi_device *device)
11961205
{
11971206
int result = 0;
@@ -1214,7 +1223,7 @@ static int acpi_battery_add(struct acpi_device *device)
12141223
if (result)
12151224
return result;
12161225

1217-
result = devm_mutex_init(&device->dev, &battery->sysfs_lock);
1226+
result = devm_mutex_init(&device->dev, &battery->update_lock);
12181227
if (result)
12191228
return result;
12201229

@@ -1244,7 +1253,7 @@ static int acpi_battery_add(struct acpi_device *device)
12441253
device_init_wakeup(&device->dev, 0);
12451254
unregister_pm_notifier(&battery->pm_nb);
12461255
fail:
1247-
sysfs_remove_battery(battery);
1256+
sysfs_battery_cleanup(battery);
12481257

12491258
return result;
12501259
}
@@ -1263,6 +1272,9 @@ static void acpi_battery_remove(struct acpi_device *device)
12631272

12641273
device_init_wakeup(&device->dev, 0);
12651274
unregister_pm_notifier(&battery->pm_nb);
1275+
1276+
guard(mutex)(&battery->update_lock);
1277+
12661278
sysfs_remove_battery(battery);
12671279
}
12681280

@@ -1280,6 +1292,9 @@ static int acpi_battery_resume(struct device *dev)
12801292
return -EINVAL;
12811293

12821294
battery->update_time = 0;
1295+
1296+
guard(mutex)(&battery->update_lock);
1297+
12831298
acpi_battery_update(battery, true);
12841299
return 0;
12851300
}

0 commit comments

Comments
 (0)