Re: [PATCH v6 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] Kernel panic due to dereferencing ZERO_SIZE_PTR when WMI methods return an empty result, bypassing length validation.
--

commit 1a33f1fa92cf5d49dd49bba56bde6f9f8debd47d
Author: Armin Wolf <W_Armin@gmx.de>

    platform/x86: dell-ddv: Use new buffer-based WMI API
    
    Use the new buffer-based WMI API to also support ACPI firmware
    implementations that do not use ACPI intergers/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..736d9b1fdcfbb 100644
> --- a/drivers/platform/x86/dell/dell-wmi-ddv.c
> +++ b/drivers/platform/x86/dell/dell-wmi-ddv.c

[ ... ]

> @@ -158,105 +164,96 @@ static const char * const fan_dock_labels[] = {
>  
>  static int dell_wmi_ddv_query_integer(struct wmi_device *wdev, enum dell_ddv_method method,
>  				      u32 arg, u32 *res)
>  {
> -	union acpi_object *obj;
> +	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(__le32));
>  	if (ret < 0)
>  		return ret;
>  
> -	if (obj->integer.value <= U32_MAX)
> -		*res = (u32)obj->integer.value;
> -	else
> -		ret = -ERANGE;
> +	__le32 *argr __free(kfree) = output.data;
>  
> -	kfree(obj);
> +	*res = le32_to_cpu(*argr);

[Severity: High]
If the ACPI evaluation succeeds but returns no object, wmidev_invoke_method()
returns 0 and sets output.data to ZERO_SIZE_PTR.

The old code explicitly checked for this:
    obj = out.pointer;
    if (!obj)
        return -ENODATA;

Does this code now dereference a ZERO_SIZE_PTR since we don't verify
that output.length > 0 before accessing *argr?

>  
> -	return ret;
> +	return 0;
>  }
>  
>  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)
>  {
> -	union acpi_object *obj;
> -	u64 buffer_size;
> +	struct dell_wmi_buffer *buffer;
> +	struct wmi_buffer output;
> +	size_t buffer_size;
>  	int ret;
>  
> -	ret = dell_wmi_ddv_query_type(wdev, method, arg, &obj, ACPI_TYPE_PACKAGE);
> +	ret = dell_wmi_ddv_query(wdev, method, arg, &output, sizeof(*buffer));
>  	if (ret < 0)
>  		return ret;
>  
> -	if (obj->package.count != 2 ||
> -	    obj->package.elements[0].type != ACPI_TYPE_INTEGER ||
> -	    obj->package.elements[1].type != ACPI_TYPE_BUFFER) {
> -		ret = -ENOMSG;
> -
> -		goto err_free;
> -	}
> -
> -	buffer_size = obj->package.elements[0].integer.value;
> -
> -	if (!buffer_size) {
> +	buffer = output.data;
> +	if (!le32_to_cpu(buffer->raw_size)) {

[Severity: High]
Does this also cause a ZERO_SIZE_PTR dereference if output.length is 0 and
output.data is ZERO_SIZE_PTR?

[ ... ]

> -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_buffer output;
> +	size_t str_size;
> +	int ret;
> +
> +	ret = dell_wmi_ddv_query(wdev, method, arg, &output, sizeof(struct wmi_string));
> +	if (ret < 0)
> +		return ret;
> +
> +	struct wmi_string *str __free(kfree) = output.data;
> +
> +	str_size = sizeof(*str) + le16_to_cpu(str->length);

[Severity: High]
Similarly, does this code trigger a ZERO_SIZE_PTR dereference of str->length
if wmidev_invoke_method() returns 0 with no object?

> +	if (str_size > output.length) {
> +		dev_warn(&wdev->dev,
> +			 FW_WARN "WMI string size (%zu) exceeds WMI buffer size (%zu)\n",
> +			 str_size, output.length);
> +		return -EMSGSIZE;
> +	}
> +
> +	return wmi_string_to_utf8s(str, buf, length);
>  }

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