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

[email protected]
Newsgroups org.kernel.vger.linux-input
Message-ID <[email protected]>
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) <[email protected]>

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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.