Re: [PATCH v5 5/9] platform/x86: dell-ddv: Use new buffer-based WMI API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The wrapper functions `dell_wmi_ddv_query_integer()`, `dell_wmi_ddv_query_buffer()`, and `dell_wmi_ddv_query_string()` unconditionally dereference the dynamically allocated WMI output buffer without validating its length, causing a fatal ZERO_SIZE_PTR dereference if the ACPI WMI method successfully evaluates but returns no data.
--

commit 5228425f4154cf6a8a761577f1a475ca16aaf5f3
Author: Armin Wolf <W_Armin@gmx.de>
platform/x86: dell-ddv: Use new buffer-based WMI API

This commit updates the dell-ddv driver to use the new buffer-based
WMI API. This allows it to support ACPI firmware implementations that
do not use ACPI integers/strings/packages for exchanging data.

> diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
> index 62e3d060f0385..c2120d8ae2988 100644
> --- a/drivers/platform/x86/dell/dell-wmi-ddv.c
> +++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
[ ... ]
> -static int dell_wmi_ddv_query_type(struct wmi_device *wdev, enum dell_ddv_method method, u32 arg,
> -				   union acpi_object **result, acpi_object_type type)
> +static int dell_wmi_ddv_query(struct wmi_device *wdev, enum dell_ddv_method method, u32 arg,
> +			      struct wmi_buffer *output, size_t min_size)
>  {
[ ... ]
> +	return wmidev_invoke_method(wdev, 0x0, method, &input, output, min_size);
>  }
>  
>  static int dell_wmi_ddv_query_integer(struct wmi_device *wdev, enum dell_ddv_method method,
>  				      u32 arg, u32 *res)
>  {
> -	union acpi_object *obj;
> +	__le32 *argr __free(kfree) = NULL;
> +	struct wmi_buffer output;
>  	int ret;
>  
> -	ret = dell_wmi_ddv_query_type(wdev, method, arg, &obj, ACPI_TYPE_INTEGER);
> +	ret = dell_wmi_ddv_query(wdev, method, arg, &output, sizeof(*argr));
>  	if (ret < 0)
>  		return ret;
>  
> -	if (obj->integer.value <= U32_MAX)
> -		*res = (u32)obj->integer.value;
> -	else
> -		ret = -ERANGE;
> -
> -	kfree(obj);
> +	argr = output.data;
> +	*res = le32_to_cpu(*argr);

[Severity: High]
Can this dereference a ZERO_SIZE_PTR and cause a crash if the ACPI method
returns no data?

Looking at wmidev_invoke_method(), if the method evaluation is successful
but no object is returned (!obj), it returns 0 (success) and sets
out->data = ZERO_SIZE_PTR, effectively bypassing the min_size check.

The previous implementation in dell_wmi_ddv_query_type() checked for this
explicitly:

	obj = out.pointer;
	if (!obj)
		return -ENODATA;

Does the new code need to verify output.length before dereferencing argr?

[ ... ]
>  static int dell_wmi_ddv_query_buffer(struct wmi_device *wdev, enum dell_ddv_method method,
> -				     u32 arg, union acpi_object **result)
> +				     u32 arg, struct dell_wmi_buffer **result)
>  {
[ ... ]
> -	if (!buffer_size) {
> +	buffer = output.data;
> +	if (!le32_to_cpu(buffer->raw_size)) {

[Severity: High]
Similarly, does this unconditionally dereference buffer->raw_size even if
output.data was set to ZERO_SIZE_PTR by wmidev_invoke_method() returning
no data?

[ ... ]
> -static int dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_method method,
> -				     u32 arg, union acpi_object **result)
> +static ssize_t dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_method method,
> +					 u32 arg, char *buf, size_t length)
>  {
> -	return dell_wmi_ddv_query_type(wdev, method, arg, result, ACPI_TYPE_STRING);
> +	struct wmi_string *str __free(kfree) = NULL;
> +	struct wmi_buffer output;
> +	size_t str_size;
> +	int ret;
> +
> +	ret = dell_wmi_ddv_query(wdev, method, arg, &output, sizeof(*str));
> +	if (ret < 0)
> +		return ret;
> +
> +	str = output.data;
> +	str_size = sizeof(*str) + le16_to_cpu(str->length);

[Severity: High]
Like the integer and buffer queries above, does this code dereference
str->length without confirming output.data is valid?

If the ACPI method succeeds but returns no object, str could be ZERO_SIZE_PTR.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605205937.530897-1-W_Armin@gmx.de?part=5