Re: [PATCH v2] platform/x86/lenovo: add Yoga Book 9 keyboard dock driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] NULL Pointer Dereference on sysfs read for QUERY device and DMI check bypass
- [High] Use-After-Free due to unreferenced QUERY WMI device pointer
- [Medium] Race condition exposing incorrect initial switch state to userspace
--

commit f26ec3cd843093c54646b94f3331c501f619e886
Author: Dave Carey <carvsdriver@gmail.com>

platform/x86/lenovo: add Yoga Book 9 keyboard dock driver

This commit adds a new driver, lenovo-yb9-kbdock, to support the detachable
keyboard dock on the Lenovo Yoga Book 9 14IAH10. It queries the BKBD state
via WMI to report SW_TABLET_MODE input events and exposes the raw position
to userspace via sysfs.

> diff --git a/drivers/platform/x86/lenovo/yb9-kbdock.c b/drivers/platform/x86/lenovo/yb9-kbdock.c
> new file mode 100644
> index 0000000000000..ba0579f7981a3
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo/yb9-kbdock.c
[ ... ]
> +static int yb9_kbdock_probe(struct wmi_device *wdev, const void *ctx)
> +{
> +	enum yb9_guid_type type = (enum yb9_guid_type)(uintptr_t)ctx;
> +	struct yb9_kbdock_priv *priv;
> +	struct input_dev *input_dev;
> +	struct wmi_device *qwdev;
> +	int err;
> +
> +	if (type == YB9_GUID_QUERY) {
> +		mutex_lock(&yb9_query_lock);
> +		yb9_query_wdev = wdev;
> +		mutex_unlock(&yb9_query_lock);
> +		return 0;
> +	}

The wmi_driver struct sets .dev_groups to yb9_kbdock_groups, which means the
keyboard_position sysfs attribute is created for all devices binding to this
driver, including the query WMI device.

If userspace reads this sysfs attribute on the query device, won't
keyboard_position_show() receive a NULL pointer from dev_get_drvdata()
and dereference it when accessing priv->bkbd?

Also, because this returns before dmi_check_system() is called, does this
allow the driver to bind to the query GUID on unsupported hardware?

> +
> +	if (!dmi_check_system(yb9_kbdock_dmi_table))
> +		return -ENODEV;
[ ... ]
> +	input_set_capability(input_dev, EV_SW, SW_TABLET_MODE);
> +
> +	err = input_register_device(input_dev);
> +	if (err) {
> +		dev_err(&wdev->dev, "failed to register input device: %d\n", err);
> +		return err;
> +	}
> +
> +	priv->query_wdev = qwdev;

Is a device link or reference count via get_device() needed here?

If the query WMI device is unbound or its parent ACPI device is hot-removed,
could priv->query_wdev become a dangling pointer that gets used in
yb9_kbdock_notify() when a WMI event fires?

> +	priv->input_dev = input_dev;
> +	dev_set_drvdata(&wdev->dev, priv);
> +
> +	yb9_kbdock_update(wdev);

Could this expose an incorrect initial switch state to userspace?

Since input_register_device() makes the device visible before
yb9_kbdock_update() queries the hardware state, userspace might
read the default laptop mode (0) even if the keyboard is detached.

Can the initial hardware state be queried and reported before registering
the input device?

> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517150224.50191-1-carvsdriver@gmail.com?part=1