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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Out-of-bounds Read / Information Disclosure in `wacom_wac_pen_serial_enforce()` due to ignored `report_size`
- [High] Out-of-bounds Read / Information Disclosure in `wacom_wac_pen_serial_enforce()` for unnumbered reports
- [High] Use-After-Free / Resource Leak in `wacom_parse_and_register()` error path
--

--- Patch [1]: [PATCH v2] HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce() ---
commit 4746f43ad261e437fb8781af2fd3818e85fdf59b
Author: Wei Jie Law <98lawweijie@gmail.com>

HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce()

This commit attempts to fix an out-of-bounds read in
wacom_wac_pen_serial_enforce() by clamping the inner loop to the minimum of
the field's maxusage and report_count. This prevents the loop from extracting
usages that have no corresponding report data.

Link: https://lore.kernel.org/linux-input/20260822120926.153849-1-98lawweijie@gmail.com/

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0eafa483b7f75..1ea8763b68a82 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -113,8 +113,10 @@ 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];
> +		unsigned int count = min(field->maxusage, field->report_count);
> +
> +		for (j = 0; j < 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 does this function still have an
out-of-bounds read because it ignores the report_size parameter?

Slightly further down in this loop, hid_field_extract() is called:

    value = hid_field_extract(hdev, raw_data+1, offset + j * size, size);

The extraction is based purely on the descriptor offsets and a hardcoded
raw_data+1 offset. If a device sends a physical report smaller than the
descriptor claims, couldn't this still read past the end of the raw_data
buffer?

[Severity: High]
This is also a pre-existing issue, but does hardcoding raw_data+1
cause an out-of-bounds read for unnumbered reports?

If a device provides an unnumbered report descriptor, the core passes the
physical payload directly without a prepended report id. Since this
function always skips the first byte by using raw_data+1, won't it read
one byte past the end of the buffer when hid_field_extract() is called?

I also noticed another pre-existing issue in this same file:

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

drivers/hid/wacom_sys.c:wacom_parse_and_register() {
    ...
fail_hw_stop:
        hid_hw_stop(hdev);
fail:
        wacom_release_resources(wacom);
        return error;
}

If device initialization fails after wacom->init_work is scheduled or timers
like idleprox_timer are armed, this error path does not appear to cancel them
before devres frees the wacom structure. When the timer or delayed work later
executes, will it access the freed wacom structure?

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