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.
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.
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 <[email protected]>
---
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
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 <[email protected]>
> ---
With above fixed,
Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>
> 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
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 <[email protected]>
>> ---
>
> With above fixed,
>
> Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>
>
>> 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
>>
>>
On 2/8/24 10:22 AM, Armin Wolf wrote:
> 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.
I think it is better to include this detail. But, since it is not a standard specification,
I will let maintainer make this call.
>
>>> 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).
>
Got it.
> 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 <[email protected]>
>>> ---
>>
>> With above fixed,
>>
>> Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>
>>
>>> 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