linux-kernel - Re: [PATCH v6] ACPI: battery: prevent sysfs_add

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0j2UH31wJKsAE0Ppek9sSiGFbePZymzbuaLVNfKHXjK4A@mail.gmail.com>
Date: Tue, 23 Sep 2025 19:12:03 +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 Tue, Sep 23, 2025 at 6:14 PM GuangFei Luo <luogf2025@....com> wrote:
>
> The functions battery_hook_add_battery(), battery_hook_remove_battery(),
> and sysfs_remove_battery() already acquire locks, so their internal
> accesses are safe.

In fact, there are two locks in use, battery->sysfs_lock and
hook_mutex.  The latter is used for managing hooks and the former is
only used by sysfs_remove_battery(), so it only prevents that function
from racing with another instance of itself.

I would suggest using battery->sysfs_lock for protecting battery->bat
in general.

> acpi_battery_refresh() does check battery->bat, but its child
> functions (sysfs_add_battery() and sysfs_remove_battery()) already
> handle locking.

What locking?  Before the $subject patch, sysfs_add_battery() doesn't
do any locking at all AFAICS.

> In acpi_battery_notify(), battery->bat has no lock. However, the
> check of battery->bat is at the very end of the function. During
> earlier calls, battery->bat has already been protected by locks, so
> re-entry will not cause issues.

All of the battery->bat checks and the code depending on them need to
go under the same lock.  I'd use battery->sysfs_lock for this as
already mentioned above.