linux-kernel - Re: [PATCH v3] 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: <2025091242-waltz-gruffly-0936@gregkh>
Date: Fri, 12 Sep 2025 17:57:19 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: GuangFei Luo <luogf2025@....com>
Cc: "Rafael J . Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org, Sebastian Reichel <sre@...nel.org>,
	linux-pm@...r.kernel.org, Dan Carpenter <dan.carpenter@...aro.org>,
	kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v3] ACPI: battery: prevent sysfs_add_battery re-entry on
 rapid events

On Fri, Sep 12, 2025 at 11:48:59PM +0800, GuangFei Luo wrote:
> 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:
> 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)

Changelog should go below the --- line.

>  static int sysfs_add_battery(struct acpi_battery *battery)
>  {
> +	mutex_lock(&battery->sysfs_lock);

Why not just use a guard() here?  That would make the logic elsewhere
much simpler.

thanks,

greg k-h