Received-SPF: pass (google.com: domain of linux-kernel+bounces-58526-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <d82f47fa-a126-4242-b4c3-83ba2c37fd95@gmx.de>
Date: Thu, 8 Feb 2024 19:22:52 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] platform/x86: wmi: Make input buffer madatory when
 evaulating methods
To: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>,
 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>
 <6360f90f-1aca-4355-aa19-661c2925dd24@linux.intel.com>
Content-Language: en-US
From: Armin Wolf <W_Armin@gmx.de>
In-Reply-To: <6360f90f-1aca-4355-aa19-661c2925dd24@linux.intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
UI-OutboundReport: notjunk:1;M01:P0:A8B7UDJo+kc=;4AstgXv6B2AcLkizu9bLEnXU+A6
 DNW2zwgGZA5ngDuLPsAukxx81WzfMum/NCPd4hHzI2QjUEUbs7LKOf3+ABDYe0bNdzVxgqejb
 qhJG0uUyDLdwSDBbWTdJnKthdaHStoDoQEWqP1egbIBzb6QyDAIESB9QkjnWEla/203zfr7Xq
 Jqao5YEQcrEpczuGAFU1SqK60ItjQF9QJb3hD2mmrqu/DEENh9XONsEOVzIKpIdBunSO7tVPG
 BVbAu328Vw8putOApHfuRij/aEUBrTPHx7/K1cpbHaVXCiBKh5r7WgX+ExsfcrFC+1sJnZm1m
 hfMe/P22IC5lWZWe/H5qKgDelksMmPOicfN+NhLMB+ODF3Oeb9vFLBwUp2HOVORY0z+RIrhwp
 kJpwHxOVRmin/XeCcxbMJfkIigHE/kriouUzICRd+KikU5ZE7y1dju8dA1+RjQZcpseJ5TKVG
 VAX537pTgisohwt7vKvTeQRIBf6VVl6xoMgHLkSEo0Ig0lq5oHYnht+zm9Y9+zjscFJgu3auL
 j6fZH00gI6pmEYnfc/JNeyXYrBBYHBtBKLonznXUNuts1Lu3QDkOv1bVcHjPqVdPzGGppEwEN
 RMpFTULk7U9+WnEegtCO/c7mESf0/XqDs11SUgAFtV/3QFHsrDziOn4ixppJTgEuI6aKgXOKP
 ufd5McR4rjcXlXBJ7/D6V9yLojxZxgRzWaiGHqDK85r0JgW8sSSHavnvMllJQhZeArmiPgyhh
 Gdj3enEpX0EUPCO8dlrJTqSJ9U1HEOr72bFMY9gnxg+zfZydD5bVZMY8XTS+ePRAVUVxPb43t
 1khYDVuj6VeAHN/LFOoLMsEUXV2H0F8E2Mpzw/iJEu+iQ=

Am 08.02.24 um 03:41 schrieb Kuppuswamy Sathyanarayanan:

> 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.

The ACPI-WMI specification is not part of the ACPI specification. It
consists of a single whitepaper published by Microsoft:

https://github.com/microsoft/Windows-driver-samples/blob/main/wmi/wmiacpi/wmi-acpi.htm

The section describing the ACPI control methods is called
"ACPI Control Method Naming Conventions and Functionality for Windows 2000 Instrumentation".

I do not thing that mentioning this in the commit message would be of any use.

>> 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?

Its not a compiler warning.

The ACPI control method is being evaluated at runtime, so the warning message
will also be generated at runtime (when interpreting the AML bytecode).

Thanks,
Armin Wolf

>> 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
>>
>>