Re: [PATCH v3 8/8] HID: wacom: do not use 'driver_data'

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Removal of driver_data breaks dynamic device binding via sysfs 'new_id', preventing testing of unknown/clone devices.

Pre-existing issues:
- [High] Missing cancellation of `wacom->init_work` on the error path for wireless monitors leads to a UAF.
--

--- Patch [8]: [PATCH v3 8/8] HID: wacom: do not use 'driver_data' ---
commit 147dba29dc34be167b7150ce20799c7b7dd39442
Author: Pawel Zalewski (The Capable Hub) <pzalewski@thegoodpenguin.co.uk>

HID: wacom: do not use 'driver_data'

This patch removes the use of driver_data for looking up product features in
the wacom driver. It instead looks up wacom_features directly by product ID
to reduce macro complexity and simplify the code.

[ ... ]

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 834519751a5cf..4514b2762cc33 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2848,7 +2845,7 @@ static int wacom_probe(struct hid_device *hdev,
>  
>  	wacom_wac = &wacom->wacom_wac;
>  
> -	const struct wacom_features *id_features = get_wacom_features(id->driver_data);
> +	const struct wacom_features *id_features = get_wacom_features(id->product);

[Severity: Critical]
Does removing driver_data introduce a regression for dynamic device binding
via sysfs 'new_id'?

By strictly using get_wacom_features(id->product), the mechanism for injecting
unknown product IDs seems rendered unusable because the injected PID won't match
any entry in the static wacom_features_table. Previously, couldn't users pass a
feature table index via driver_data to test unknown or clone devices?

[Severity: High]
This is a pre-existing issue, but is there a missing cancellation of 
wacom->init_work on the error path for wireless monitors?

drivers/hid/wacom_sys.c:wacom_parse_and_register() {
    ...
	if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
		error = hid_hw_open(hdev);
		if (error) {
			hid_err(hdev, "hw open failed\n");
			goto fail_hw_stop;
		}
	}
    ...
}

In wacom_parse_and_register(), wacom_query_tablet_data() schedules
wacom->init_work. If hid_hw_open() subsequently fails for a wireless monitor
(WACOM_DEVICETYPE_WL_MONITOR), the error path jumps to fail_hw_stop and
returns without calling cancel_delayed_work_sync(&wacom->init_work).

Because the wacom structure is managed by devres and will be freed upon probe
failure, could this result in a use-after-free when init_work executes later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-mod-devicetable-hid_device_id-v3-0-980896ca283e@thegoodpenguin.co.uk?part=8