| 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: <CAJZ5v0hN52GMte2dCx1u=i_MSuLFMt+2-HYdg+nmU73U9WptDQ@mail.gmail.com>
Date: Mon, 22 Sep 2025 19:29:48 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: GuangFei Luo <luogf2025@....com>
Cc: rafael@...nel.org, michal.wilczynski@...el.com, dan.carpenter@...aro.org,
lenb@...nel.org, linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org, lkp@...el.com, sre@...nel.org,
stable@...r.kernel.org
Subject: Re: [PATCH v6] ACPI: battery: prevent sysfs_add_battery re-entry on
rapid events
On Thu, Sep 18, 2025 at 3:56 PM GuangFei Luo <luogf2025@....com> wrote:
>
> When removing and reinserting the laptop battery, ACPI can trigger
> two notifications in quick succession:
> - ACPI_BATTERY_NOTIFY_STATUS (0x80)
> - ACPI_BATTERY_NOTIFY_INFO (0x81)
>
> Both notifications call acpi_battery_update(). Because the events
> happen very close in time, sysfs_add_battery() can be re-entered
> before battery->bat is set, causing a duplicate sysfs entry error.
>
> When the ACPI battery driver uses
> acpi_dev_install_notify_handler() to register acpi_battery_notify,
> the callback may be triggered twice in a very short period of time.
>
> This patch ensures that sysfs_add_battery() is not re-entered
> when battery->bat is already non-NULL, preventing the duplicate
> sysfs creation and stabilizing battery hotplug handling.
>
> [ 476.117945] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1'
> [ 476.118896] 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
> [ 476.118903] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009
> [ 476.118906] Workqueue: kacpi_notify acpi_os_execute_deferred
> [ 476.118917] Call Trace:
> [ 476.118922] <TASK>
> [ 476.118929] dump_stack_lvl+0x5d/0x80
> [ 476.118938] sysfs_warn_dup.cold+0x17/0x23
> [ 476.118943] sysfs_create_dir_ns+0xce/0xe0
> [ 476.118952] kobject_add_internal+0xba/0x250
> [ 476.118959] kobject_add+0x96/0xc0
> [ 476.118964] ? get_device_parent+0xde/0x1e0
> [ 476.118970] device_add+0xe2/0x870
> [ 476.118975] __power_supply_register.part.0+0x20f/0x3f0
> [ 476.118981] ? wake_up_q+0x4e/0x90
> [ 476.118990] sysfs_add_battery+0xa4/0x1d0 [battery]
> [ 476.118998] acpi_battery_update+0x19e/0x290 [battery]
> [ 476.119002] acpi_battery_notify+0x50/0x120 [battery]
> [ 476.119006] acpi_ev_notify_dispatch+0x49/0x70
> [ 476.119012] acpi_os_execute_deferred+0x1a/0x30
> [ 476.119015] process_one_work+0x177/0x330
> [ 476.119022] worker_thread+0x251/0x390
> [ 476.119026] ? __pfx_worker_thread+0x10/0x10
> [ 476.119030] kthread+0xd2/0x100
> [ 476.119033] ? __pfx_kthread+0x10/0x10
> [ 476.119035] ret_from_fork+0x34/0x50
> [ 476.119040] ? __pfx_kthread+0x10/0x10
> [ 476.119042] ret_from_fork_asm+0x1a/0x30
> [ 476.119049] </TASK>
> [ 476.142552] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
> [ 476.415022] ata1.00: unexpected _GTF length (8)
> [ 476.428076] sd 0:0:0:0: [sda] Starting disk
> [ 476.835035] ata1.00: unexpected _GTF length (8)
> [ 476.839720] ata1.00: configured for UDMA/133
> [ 491.328831] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1'
> [ 491.329720] 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
> [ 491.329727] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009
> [ 491.329731] Workqueue: kacpi_notify acpi_os_execute_deferred
> [ 491.329741] Call Trace:
> [ 491.329745] <TASK>
> [ 491.329751] dump_stack_lvl+0x5d/0x80
> [ 491.329758] sysfs_warn_dup.cold+0x17/0x23
> [ 491.329762] sysfs_create_dir_ns+0xce/0xe0
> [ 491.329770] kobject_add_internal+0xba/0x250
> [ 491.329775] kobject_add+0x96/0xc0
> [ 491.329779] ? get_device_parent+0xde/0x1e0
> [ 491.329784] device_add+0xe2/0x870
> [ 491.329790] __power_supply_register.part.0+0x20f/0x3f0
> [ 491.329797] sysfs_add_battery+0xa4/0x1d0 [battery]
> [ 491.329805] acpi_battery_update+0x19e/0x290 [battery]
> [ 491.329809] acpi_battery_notify+0x50/0x120 [battery]
> [ 491.329812] acpi_ev_notify_dispatch+0x49/0x70
> [ 491.329817] acpi_os_execute_deferred+0x1a/0x30
> [ 491.329820] process_one_work+0x177/0x330
> [ 491.329826] worker_thread+0x251/0x390
> [ 491.329830] ? __pfx_worker_thread+0x10/0x10
> [ 491.329833] kthread+0xd2/0x100
> [ 491.329836] ? __pfx_kthread+0x10/0x10
> [ 491.329838] ret_from_fork+0x34/0x50
> [ 491.329842] ? __pfx_kthread+0x10/0x10
> [ 491.329844] ret_from_fork_asm+0x1a/0x30
> [ 491.329850] </TASK>
> [ 491.329855] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
>
> Fixes: 10666251554c ("ACPI: battery: Install Notify() handler directly")
> Signed-off-by: GuangFei Luo <luogf2025@....com>
> Cc: stable@...r.kernel.org
> ---
> v6:
> - Update Fixes tag: point to commit 10666251554c ("ACPI: battery: Install
> Notify() handler directly"), which introduced the sysfs_add_battery()
> re-entry issue when acpi_battery_notify is registered via
> acpi_dev_install_notify_handler(). The problem does not occur with
> acpi_bus_register_driver().
>
> v5:
> - Move changelog above the '---' line as per submission guidelines.
>
> v4:
> - Uses guard(mutex) for battery->sysfs_lock in sysfs_add_battery().
> - Since sysfs_add_battery() now handles the battery->bat check with
> proper locking,the extra if (!battery->bat) check at the call site
> has become redundant.
>
> v3:
> - Modified the earlier approach: since sysfs_add_battery() is invoked
> from multiple places, the most reliable way is to add the lock inside
> the function itself.
> - sysfs_remove_battery() had a similar race issue in the past, which was
> fixed by adding a lock as well. Reference:
> https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669
> .1312318300.git.len.brown@...el.com/
>
> v2:
> - Fix missing mutex_unlock in acpi_battery_update()
> (Reported-by: kernel test robot)
>
> v1:
> - Initial patch to handle race when hotplugging battery, preventing
> duplicate sysfs entries.
> ---
> drivers/acpi/battery.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 6905b56bf3e4..20d68f3e881f 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -850,6 +850,10 @@ static void __exit battery_hook_exit(void)
>
> static int sysfs_add_battery(struct acpi_battery *battery)
> {
> + guard(mutex)(&battery->sysfs_lock);
> + if (battery->bat)
> + return 0;
> +
> struct power_supply_config psy_cfg = {
> .drv_data = battery,
> .attr_grp = acpi_battery_groups,
> @@ -1026,11 +1030,9 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume)
> return result;
> acpi_battery_quirks(battery);
>
> - if (!battery->bat) {
> - result = sysfs_add_battery(battery);
> - if (result)
> - return result;
> - }
> + result = sysfs_add_battery(battery);
> + if (result)
> + return result;
>
> /*
> * Wakeup the system if battery is critical low
> @@ -1112,12 +1114,12 @@ static int battery_notify(struct notifier_block *nb,
> result = acpi_battery_get_info(battery);
> if (result)
> return result;
> -
> - result = sysfs_add_battery(battery);
> - if (result)
> - return result;
> }
>
> + result = sysfs_add_battery(battery);
> + if (result)
> + return result;
> +
Why is this change necessary?
> acpi_battery_init_alarm(battery);
> acpi_battery_get_state(battery);
> break;
> --
Powered by blists - more mailing lists