linux-kernel - Re: [PATCH v1 02/10] platform/x86: msi-wmi-platform: Add unlocked msi_wmi_platform

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: <487100c4-f507-426a-b6fd-58a4cefced47@gmx.de>
Date: Tue, 13 May 2025 21:47:12 +0200
From: Armin Wolf <W_Armin@....de>
To: Antheas Kapenekakis <lkml@...heas.dev>,
 platform-driver-x86@...r.kernel.org
Cc: Jonathan Corbet <corbet@....net>, Hans de Goede <hdegoede@...hat.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 Jean Delvare <jdelvare@...e.com>, Guenter Roeck <linux@...ck-us.net>,
 Kurt Borja <kuurtb@...il.com>, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v1 02/10] platform/x86: msi-wmi-platform: Add unlocked
 msi_wmi_platform_query

Am 11.05.25 um 22:44 schrieb Antheas Kapenekakis:

> This driver requires to be able to handle transactions that perform
> multiple WMI actions at a time. Therefore, it needs to be able to
> lock the wmi_lock mutex for multiple operations.
>
> Add msi_wmi_platform_query_unlocked() to allow the caller to
> perform the WMI query without locking the wmi_lock mutex, by
> renaming the existing function and adding a new one that only
> locks the mutex.
>
> Signed-off-by: Antheas Kapenekakis <lkml@...heas.dev>
> ---
>   drivers/platform/x86/msi-wmi-platform.c | 27 ++++++++++++++++---------
>   1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/x86/msi-wmi-platform.c b/drivers/platform/x86/msi-wmi-platform.c
> index 41218a9d6e35d..f0d1b8e1a2fec 100644
> --- a/drivers/platform/x86/msi-wmi-platform.c
> +++ b/drivers/platform/x86/msi-wmi-platform.c
> @@ -140,7 +140,7 @@ static int msi_wmi_platform_parse_buffer(union acpi_object *obj, u8 *output, siz
>   	return 0;
>   }
>   
> -static int msi_wmi_platform_query(struct msi_wmi_platform_data *data,
> +static int msi_wmi_platform_query_unlocked(struct msi_wmi_platform_data *data,
>   				  enum msi_wmi_platform_method method, u8 *buffer,
>   				  size_t length)
>   {
> @@ -156,15 +156,9 @@ static int msi_wmi_platform_query(struct msi_wmi_platform_data *data,
>   	if (!length)
>   		return -EINVAL;
>   
> -	/*
> -	 * The ACPI control method responsible for handling the WMI method calls
> -	 * is not thread-safe. Because of this we have to do the locking ourself.
> -	 */
> -	scoped_guard(mutex, &data->wmi_lock) {
> -		status = wmidev_evaluate_method(data->wdev, 0x0, method, &in, &out);
> -		if (ACPI_FAILURE(status))
> -			return -EIO;
> -	}
> +	status = wmidev_evaluate_method(data->wdev, 0x0, method, &in, &out);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
>   
>   	obj = out.pointer;
>   	if (!obj)
> @@ -176,6 +170,19 @@ static int msi_wmi_platform_query(struct msi_wmi_platform_data *data,
>   	return ret;
>   }
>   
> +static int msi_wmi_platform_query(struct msi_wmi_platform_data *data,
> +				  enum msi_wmi_platform_method method, u8 *buffer,
> +				  size_t length)
> +{
> +	/*
> +	 * The ACPI control method responsible for handling the WMI method calls
> +	 * is not thread-safe. Because of this we have to do the locking ourself.
> +	 */
> +	scoped_guard(mutex, &data->wmi_lock) {
> +		return msi_wmi_platform_query_unlocked(data, method, buffer, length);
> +	}

Please just use guard() here.

Thanks,
Armin Wolf

> +}
> +
>   static umode_t msi_wmi_platform_is_visible(const void *drvdata, enum hwmon_sensor_types type,
>   					   u32 attr, int channel)
>   {