Re: [PATCH v6 3/9] platform/x86: dell-smbios-wmi: Use new buffer-based WMI API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] If the ACPI WMI method successfully evaluates but returns no output object, `wmidev_invoke_method()` returns success and sets the output data to `ZERO_SIZE_PTR`, bypassing the `min_size` constraint. `run_smbios_call()` then unconditionally executes `memcpy` from `ZERO_SIZE_PTR`, leading to a kernel panic.
- [High] Use-After-Free (UAF) in the ioctl and read paths because the character device is unregistered using a devm action, allowing file operations to execute after the underlying device memory is freed.
--

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

platform/x86: dell-smbios-wmi: Use new buffer-based WMI API

This commit updates the dell-smbios-wmi driver to use the new buffer-based WMI
API. It transitions the driver to support ACPI firmware implementations that do
not rely on ACPI buffers for SMBIOS call results.

> diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
> index a7dca8c59d602..64d0871b706e5 100644
> --- a/drivers/platform/x86/dell/dell-smbios-wmi.c
> +++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
> @@ -50,38 +50,32 @@ static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
>  
>  static int run_smbios_call(struct wmi_device *wdev)
>  {

[ ... ]

> -	status = wmidev_evaluate_method(wdev, 0, 1, &input, &output);
> -	if (ACPI_FAILURE(status))
> -		return -EIO;
> -	obj = (union acpi_object *)output.pointer;
> -	if (obj->type != ACPI_TYPE_BUFFER) {
> -		dev_dbg(&wdev->dev, "received type: %d\n", obj->type);
> -		if (obj->type == ACPI_TYPE_INTEGER)
> -			dev_dbg(&wdev->dev, "SMBIOS call failed: %llu\n",
> -				obj->integer.value);
> -		kfree(output.pointer);
> -		return -EIO;
> -	}
> -	memcpy(input.pointer, obj->buffer.pointer, obj->buffer.length);
> +	/*
> +	 * The output buffer returned by the WMI method should have at least the size
> +	 * of the input buffer.
> +	 */
> +	ret = wmidev_invoke_method(wdev, 0, 1, &input, &output, input.length);
> +	if (ret < 0)
> +		return ret;
> +
> +	memcpy(input.data, output.data, input.length);

[Severity: High]
This is a pre-existing issue, but can this memcpy() cause a kernel panic
if the WMI method successfully evaluates but returns no output object?

If wmidev_invoke_method() in drivers/platform/wmi/core.c encounters a
missing ACPI object (!obj), it returns 0 and sets out->data to ZERO_SIZE_PTR:

    status = wmidev_evaluate_method(wdev, instance, method_id, &ain, &aout);
    ...
    obj = aout.pointer;
    if (!obj) {
        out->length = 0;
        out->data = ZERO_SIZE_PTR;
        return 0;
    }

Since wmidev_invoke_method() returns success and bypasses the min_size check,
the memcpy() here would attempt to read input.length bytes from ZERO_SIZE_PTR,
leading to an invalid memory access.

[Severity: High]
This is a pre-existing issue, but does the character device unregistration
sequence allow for a use-after-free during device unbind?

The driver registers a character device via misc_register() and uses
devm_add_action_or_reset() to unregister it. It also allocates the priv
and priv->buf structures using devm_kzalloc() and devm_get_free_pages().

When the WMI device is removed (e.g., via sysfs unbind), the devm cleanup
actions call misc_deregister() and then free the priv memory. However,
misc_deregister() does not revoke existing open file descriptors.

If a userspace process holds the character device open while the device
is unbound, it can subsequently call ioctl() or read(). These operations
would dereference filp->private_data in dell_smbios_wmi_ioctl() (which
points to the freed priv struct) and access the freed priv->buf memory.

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