Re: [PATCH v6 4/9] platform/x86: dell-wmi-base: 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] An out-of-bounds read occurs in `dell_wmi_process_key()` when processing an Eprivacy toggle WMI event with a truncated payload.
- [Medium] The dynamically allocated `tabletswitch_dev` input device does not have its `dev.parent` field set before registration, violating input subsystem hierarchy guidelines.
--

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

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

This commit updates the driver to use the new buffer-based WMI API.
This also supports ACPI firmware implementations that do not use ACPI
buffers for the event data.

> diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
> index 2a5804efd3ea8..997383ba18465 100644
> --- a/drivers/platform/x86/dell/dell-wmi-base.c
> +++ b/drivers/platform/x86/dell/dell-wmi-base.c

[ ... ]

> @@ -446,15 +448,15 @@ static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16
>  	} else if (type == 0x0011 && code == 0xe070 && remaining > 0) {
>  		dell_wmi_switch_event(&priv->tabletswitch_dev,
>  				      "Dell tablet mode switch",
> -				      SW_TABLET_MODE, !buffer[0]);
> +				      SW_TABLET_MODE, !le16_to_cpu(buffer[0]));

[Severity: Medium]
This isn't a bug introduced by this patch, but does dell_wmi_switch_event()
register the input device without assigning dev->dev.parent?

Looking at the dell_wmi_switch_event() definition:

	if (!*subdev) {
		struct input_dev *dev = input_allocate_device();
		...
		(dev)->name = devname;
		(dev)->id.bustype = BUS_HOST;
		if (input_register_device(dev)) {

Without explicitly setting dev->dev.parent before registration, the device
might appear under /sys/devices/virtual/input/ instead of correctly rooting
in the sysfs hierarchy.

>  		return 1;
>  	} else if (type == 0x0012 && code == 0x000c && remaining > 0) {
>  		/* Eprivacy toggle, switch to "on" key entry for on events */
> -		if (buffer[0] == 2)
> +		if (le16_to_cpu(buffer[0]) == 2)
>  			key++;
>  		used = 1;
>  	} else if (type == 0x0012 && code == 0x000d && remaining > 0) {
> -		value = (buffer[2] == 2);
> +		value = (le16_to_cpu(buffer[2]) == 2);

[Severity: High]
This is a pre-existing issue, but does this cause an out-of-bounds read
when the payload is truncated?

Since remaining > 0 only guarantees that buffer[0] is accessible, could
accessing buffer[2] read memory past the end of the validated WMI buffer
if the remaining event length is 1 or 2?

>  		used = 1;
>  	}
>

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