Re: [PATCH v4] HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free in wacom_parse_and_register() error path
- [High] Race condition leading to Use-After-Free during concurrent unbind
--

commit 63c430d20632598184f0ad0353c6bd9e43f51d20
Author: Wei Jie LAW <98lawweijie@gmail.com>
HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce()

This commit fixes an out-of-bounds read in wacom_wac_pen_serial_enforce()
by clamping the usage loop to field->report_count instead of maxusage.
This prevents calculating an offset beyond the end of the report, which
could otherwise lead to an information disclosure.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0eafa483b7f75..40770affdbdee 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -113,8 +113,9 @@ static int wacom_wac_pen_serial_enforce(struct hid_device *hdev,
>  
>  	/* Queue events which have invalid tool type or serial number */
>  	for (i = 0; i < report->maxfield; i++) {
> -		for (j = 0; j < report->field[i]->maxusage; j++) {
> -			struct hid_field *field = report->field[i];
> +		struct hid_field *field = report->field[i];
> +
> +		for (j = 0; j < field->report_count; j++) {
>  			struct hid_usage *usage = &field->usage[j];
>  			unsigned int equivalent_usage = wacom_equivalent_usage(usage->hid);
>  			unsigned int offset;

[Severity: High]
This is a pre-existing issue, but can this lead to a use-after-free in the
wacom_parse_and_register() error path?

If hid_hw_open() fails for a WL_MONITOR device:

drivers/hid/wacom_sys.c:wacom_parse_and_register() {
    ...
	if (!wireless) {
		/* Note that if query fails it is not a hard failure */
		wacom_query_tablet_data(wacom);
	}
    ...
	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;
		}
	}
    ...
fail_hw_stop:
	hid_hw_stop(hdev);
fail:
	wacom_release_resources(wacom);
	return error;
}

The init_work is scheduled by wacom_query_tablet_data(), but the error
path for hid_hw_open() fails to cancel it. When the probe function returns
an error, the driver core releases devres resources and frees the wacom
structure. Will the pending work access the freed structure if it
executes later?

[Severity: High]
This is a pre-existing issue, but is there a race condition leading to a
use-after-free during concurrent unbind in wacom_mode_change_work()?

drivers/hid/wacom_sys.c:wacom_mode_change_work() {
    ...
	if (shared->pen) {
		wacom1 = hid_get_drvdata(shared->pen);
		wacom_release_resources(wacom1);
		hid_hw_stop(wacom1->hdev);
    ...
	}

	if (shared->touch) {
		wacom2 = hid_get_drvdata(shared->touch);
		wacom_release_resources(wacom2);
		hid_hw_stop(wacom2->hdev);
    ...
}

If mode_change_work runs on one interface (e.g. pen) while the sibling
interface (e.g. touch) is concurrently unbound, it reads shared->touch
locklessly and retrieves the sibling's wacom struct.

If this thread is preempted and the sibling finishes unbinding, devres will
free its wacom struct. When mode_change_work resumes and calls
wacom_release_resources() on the freed structure, could this result in a
use-after-free?

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