Received-SPF: pass (google.com: domain of linux-kernel+bounces-57397-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <6360f90f-1aca-4355-aa19-661c2925dd24@linux.intel.com>
Date: Wed, 7 Feb 2024 18:41:59 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] platform/x86: wmi: Make input buffer madatory when
 evaulating methods
Content-Language: en-US
To: Armin Wolf <W_Armin@gmx.de>, hdegoede@redhat.com,
 ilpo.jarvinen@linux.intel.com
Cc: platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240207202012.3506-1-W_Armin@gmx.de>
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
In-Reply-To: <20240207202012.3506-1-W_Armin@gmx.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 2/7/24 12:20 PM, Armin Wolf wrote:
> The ACPI-WMI specification declares that a WMxx control method takes
> 3 arguments: instance, method id and argument buffer. This is also
> the case even when the underlying WMI method does not have any
> input arguments.

It would be better if you include specification version and section
title for reference.

>
> So if a WMI driver evaluates a WMI method without passing an input
> buffer, ACPICA will log a warning complaining that the third argument
> is missing.

I assume it is a compile warning. Can you copy the warning message?

>
> Prevent this by checking that a input buffer was passed, and return
> an error if this was not the case.
>
> Tested on a Asus PRIME B650-Plus.
>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---


With above fixed,

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

>  drivers/platform/x86/wmi.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index 63906fdd0abf..f9e23d491dd9 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -296,7 +296,7 @@ EXPORT_SYMBOL_GPL(wmidev_instance_count);
>   * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
>   * @instance: Instance index
>   * @method_id: Method ID to call
> - * @in: Buffer containing input for the method call
> + * @in: Mandatory buffer containing input for the method call
>   * @out: Empty buffer to return the method results
>   *
>   * Call an ACPI-WMI method, the caller must free @out.
> @@ -326,7 +326,7 @@ EXPORT_SYMBOL_GPL(wmi_evaluate_method);
>   * @wdev: A wmi bus device from a driver
>   * @instance: Instance index
>   * @method_id: Method ID to call
> - * @in: Buffer containing input for the method call
> + * @in: Mandatory buffer containing input for the method call
>   * @out: Empty buffer to return the method results
>   *
>   * Call an ACPI-WMI method, the caller must free @out.
> @@ -347,26 +347,25 @@ acpi_status wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, u32 met
>  	block = &wblock->gblock;
>  	handle = wblock->acpi_device->handle;
>
> +	if (!in)
> +		return AE_BAD_DATA;
> +
>  	if (!(block->flags & ACPI_WMI_METHOD))
>  		return AE_BAD_DATA;
>
>  	if (block->instance_count <= instance)
>  		return AE_BAD_PARAMETER;
>
> -	input.count = 2;
> +	input.count = 3;
>  	input.pointer = params;
> +
>  	params[0].type = ACPI_TYPE_INTEGER;
>  	params[0].integer.value = instance;
>  	params[1].type = ACPI_TYPE_INTEGER;
>  	params[1].integer.value = method_id;
> -
> -	if (in) {
> -		input.count = 3;
> -
> -		params[2].type = get_param_acpi_type(wblock);
> -		params[2].buffer.length = in->length;
> -		params[2].buffer.pointer = in->pointer;
> -	}
> +	params[2].type = get_param_acpi_type(wblock);
> +	params[2].buffer.length = in->length;
> +	params[2].buffer.pointer = in->pointer;
>
>  	get_acpi_method_name(wblock, 'M', method);
>
> --
> 2.39.2
>
>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer