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

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

[Wei Jie Law]

wacom_wac_pen_serial_enforce() iterates over a field's usages (up to
field->maxusage) but indexes the report bits by j * report_size:

	for (i = 0; i < report->maxfield; i++) {
		for (j = 0; j < report->field[i]->maxusage; j++) {
			...
			value = hid_field_extract(hdev, raw_data + 1,
						  offset + j * size, size);

In hid_add_field() the usage array is sized max(usage_index,
report_count), so field->maxusage can be far larger than report_count
when a descriptor lists more usages than its Report Count.  A field
declaring Usage Minimum 0 / Usage Maximum 0x2ffe with Report Count 1
gives maxusage == 12288 while the field's bit region is only 8 bits
wide.  The extract at j == 12287 then reads bit offset 12287 * 8, i.e.
byte 12287 of raw_data + 1, roughly 12 KB past a 2-byte received
report.  __extract() performs no bounds check.

The value read that way is stored into wacom_wac->serial[0] and can be
emitted to userspace as an MSC_SERIAL event by wacom_wac_pen_report(),
so this is an information disclosure and not just an out-of-bounds
read.  A malicious device only has to claim vendor id 0x056a for
hid_scan_report() to place it in HID_GROUP_WACOM and have this driver
bound to it, and a single crafted input report is enough to trigger the
read.

Clamp the inner loop to the field's actual report region with
min(maxusage, report_count), so usages that have no report data behind
them are no longer extracted.

Verified on v6.12.104, whose wacom_sys.c is identical to mainline here,
with a UHID reproducer and with an emulated USB device (raw-gadget): a
hardware watchpoint on wacom_wac->serial[0] fires with an out-of-bounds
heap byte while a 2-byte report is being processed, and no longer fires
once the loop is clamped.

Fixes: 83417206427b ("HID: wacom: Queue events with missing type/serial data for later processing")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Assisted-by: GLM:glm-5.3
Signed-off-by: Wei Jie Law <98lawweijie@gmail.com>
---
Changes in v2:
 - No code change: the diff is identical to v1.  Adds the Assisted-by
   tags required by Documentation/process/coding-assistants.rst.

The reproducer is available on request.

Compile-tested on bd5f485f3f02, x86_64 defconfig + CONFIG_HID_WACOM=y.

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

 drivers/hid/wacom_sys.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0eafa483b7f7..1ea8763b68a8 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;
-- 
2.43.0

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

[sashiko-bot]

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

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

[Jason Gerecke]

On Tue, Aug 25, 2026 at 3:31 AM Wei Jie Law <98lawweijie@gmail.com> wrote:
>
> wacom_wac_pen_serial_enforce() iterates over a field's usages (up to
> field->maxusage) but indexes the report bits by j * report_size:
>
>         for (i = 0; i < report->maxfield; i++) {
>                 for (j = 0; j < report->field[i]->maxusage; j++) {
>                         ...
>                         value = hid_field_extract(hdev, raw_data + 1,
>                                                   offset + j * size, size);
>
> In hid_add_field() the usage array is sized max(usage_index,
> report_count), so field->maxusage can be far larger than report_count
> when a descriptor lists more usages than its Report Count.  A field

The wording above confused me greatly until I realized that the
problem is **not** with the field->usage array. That particular array
is allocated with the size described, which is exactly equal to
field->maxusage. **Instead**, the problem is that a buggy report
descriptor can declare many more usages (and thus a greater value of
field->maxusage) than the Report Count actually provides space in the
report for. In such an instance, we would obviously overstep
arbitrarily far past the end of the item.

I would appreciate an update to this patch with less confusing wording.

> declaring Usage Minimum 0 / Usage Maximum 0x2ffe with Report Count 1
> gives maxusage == 12288 while the field's bit region is only 8 bits
> wide.  The extract at j == 12287 then reads bit offset 12287 * 8, i.e.
> byte 12287 of raw_data + 1, roughly 12 KB past a 2-byte received
> report.  __extract() performs no bounds check.
>
> The value read that way is stored into wacom_wac->serial[0] and can be
> emitted to userspace as an MSC_SERIAL event by wacom_wac_pen_report(),
> so this is an information disclosure and not just an out-of-bounds
> read.  A malicious device only has to claim vendor id 0x056a for
> hid_scan_report() to place it in HID_GROUP_WACOM and have this driver
> bound to it, and a single crafted input report is enough to trigger the
> read.
>
> Clamp the inner loop to the field's actual report region with
> min(maxusage, report_count), so usages that have no report data behind
> them are no longer extracted.
>

I don't believe there is any situation where maxusage could be less
than report_count since it is initialized as max(usage_index,
report_count). We would also *want* to read fields beyond the
last-declared usage, since such fields are actually supported by HID
(see both the comment in the usages loop of hid_add_field() as well as
the first remark under section 6.2.2.8 "Local Items" of version 1.11
of the HID spec --- such fields just reuse the last-declared usage).

Given the above, I propose simply making the loop condition " j <
field->report_count". Thoughts?

Jason (she/they)
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

> Verified on v6.12.104, whose wacom_sys.c is identical to mainline here,
> with a UHID reproducer and with an emulated USB device (raw-gadget): a
> hardware watchpoint on wacom_wac->serial[0] fires with an out-of-bounds
> heap byte while a 2-byte report is being processed, and no longer fires
> once the loop is clamped.
>
> Fixes: 83417206427b ("HID: wacom: Queue events with missing type/serial data for later processing")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Assisted-by: GLM:glm-5.3
> Signed-off-by: Wei Jie Law <98lawweijie@gmail.com>
> ---
> Changes in v2:
>  - No code change: the diff is identical to v1.  Adds the Assisted-by
>    tags required by Documentation/process/coding-assistants.rst.
>
> The reproducer is available on request.
>
> Compile-tested on bd5f485f3f02, x86_64 defconfig + CONFIG_HID_WACOM=y.
>
> v1: https://lore.kernel.org/linux-input/20260822120926.153849-1-98lawweijie@gmail.com/
>
>  drivers/hid/wacom_sys.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0eafa483b7f7..1ea8763b68a8 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;
> --
> 2.43.0
>
>

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

[Wei Jie LAW]

From: Wei Jie Law <98lawweijie@gmail.com>

wacom_wac_pen_serial_enforce() iterates over a field's usages (up to
field->maxusage) but indexes the report bits by j * report_size, the
position of value slot j -- and only the field->report_count value
slots reserved by the Report Count exist in the report:

	for (i = 0; i < report->maxfield; i++) {
		for (j = 0; j < report->field[i]->maxusage; j++) {
			...
			value = hid_field_extract(hdev, raw_data + 1,
						  offset + j * size, size);

hid_add_field() sizes the usage array with max(usage_index,
report_count), so a report descriptor can declare far more usages
than its Report Count reserves room for in the report.  One listing
12288 usages against Report Count 1 has the loop extract the usage
at index 12287 from bit offset 98296 -- about 12 KB past a 2-byte
received report.  The value is stored in wacom_wac->serial[0] and
can reach userspace as an MSC_SERIAL event, making this an
information disclosure.

Clamp the loop to field->report_count, the number of value slots the
report holds. Value slots past the last declared usage are still
scanned; they reuse that usage (HID 1.11, 6.2.2.8).

Verified on v6.12.105 with a UHID reproducer: a 2-byte report from
such a descriptor trips KASAN before the patch and not after it.

Fixes: 83417206427b ("HID: wacom: Queue events with missing type/serial data for later processing")
Suggested-by: Jason Gerecke <killertofu@gmail.com>
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Assisted-by: GLM:glm-5.3
Signed-off-by: Wei Jie Law <98lawweijie@gmail.com>
---
Changes in v3, per Jason Gerecke's review:

 - reworded the explanation: the usage array is sized for maxusage;
   the descriptor just declares more usages than its Report Count
   reserves room for in the report, and the loop trusted maxusage
   over report_count

 - bound inner loop to field->report_count

The reproducer is available on request.

Compile-tested on 6ba2c27cb9aa (x86_64, wacom_sys.o); the runtime
verification used the identical wacom_sys.c built as a module for
v6.12.105.

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

 drivers/hid/wacom_sys.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0eafa483b7f7..40770affdbde 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;
-- 
2.43.0

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Delayed work 'init_work' is scheduled but not canceled on hid_hw_open() failure, leading to Use-After-Free.
- [High] Sysfs handlers wacom_led_select_store and wacom_led_select_show do not check if wacom->led.groups is NULL before accessing it, leading to a crash during unbind.
--

commit e33281ca5de90c5a4993fe522ad3163d519fa468
Author: Wei Jie Law <98lawweijie@gmail.com>

HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce()

This commit clamps the iteration loop in wacom_wac_pen_serial_enforce() to
field->report_count to prevent an out-of-bounds read when processing usages.
It fixes an information disclosure bug where reports could read past their
bounds into other memory.

[ ... ]

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

[Severity: High]
This is a pre-existing issue, and was not introduced by the patch under
review, but does this error path fail to cancel the previously scheduled
init_work before returning?

A device connects and wacom_probe() calls wacom_parse_and_register().
The function schedules wacom->init_work.

If the device is a WL_MONITOR and hid_hw_open() fails, the function exits
with an error without canceling the delayed work. 

The HID core's devres cleanup frees the wacom structure, leaving the delayed
work queued.

Could this cause a Use-After-Free memory corruption when the delayed work
executes after the wacom structure has been freed?

A malicious USB device or a fuzzer could simulate a device that fails
hid_hw_open(), reliably triggering the use-after-free.

[ ... ]

drivers/hid/wacom_sys.c:wacom_led_select_store()
> 	wacom->led.groups[set_id].select = id & 0x3;

[Severity: High]
This is a pre-existing issue, and was not introduced by the patch under
review, but could this assignment cause a NULL pointer dereference of
wacom->led.groups?

During device unbind, wacom_remove() clears wacom->led.groups to NULL. Because
the sysfs attributes were created using devres (sysfs_create_group), they are
not removed until after wacom_remove() completes. 

During this window, any read or write to status_ledX_select will invoke
wacom_led_select_show() or wacom_led_select_store(), which unconditionally
dereference wacom->led.groups.

drivers/hid/wacom_sys.c:wacom_led_select_show()
> 	return scnprintf(buf, PAGE_SIZE, "%d\n",			\
> 			 wacom->led.groups[SET_ID].select);		\

[Severity: High]
This is a pre-existing issue, and was not introduced by the patch under
review, but is wacom->led.groups accessed here without first checking if it
is NULL?

Could this lead to a NULL pointer dereference and a kernel crash?

The sysfs files are world-readable (DEV_ATTR_RW_PERM), meaning an unprivileged
local user can continuously read them in a loop while the device is being
unplugged to win the race and crash the system.

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

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

[Wei Jie Law]

On Thu, Aug 27, 2026 at 12:02 PM Jason Gerecke <killertofu@gmail.com> wrote:
> I would appreciate an update to this patch with less confusing wording.

Reworded in v3:
https://lore.kernel.org/all/20260828033323.82958-1-98lawweijie@gmail.com/

> I don't believe there is any situation where maxusage could be less
> than report_count since it is initialized as max(usage_index,
> report_count).

You are right. Thanks for the correction.

> We would also *want* to read fields beyond the last-declared usage

Agreed; the submitted v3 keeps that, and the changelog 
notes the 6.2.2.8 reuse rule.

> Given the above, I propose simply making the loop condition " j <
> field->report_count". Thoughts?

That is v3, with Suggested-by: you.  Retested on v6.12.105 with a
kprobe on hid_field_extract(): with your condition the 12288-usage /
Report Count 1 descriptor extracts nothing in the driver loop (the one
remaining call is the core's own in-bounds fetch) and trips no KASAN;
stock extracts at bit offset 98296 and does.

As a side note, I am also looking into a separate but related read in 
the same function based on Sashiko's review. I do not want to hold 
up this patch over it or speculate before I have it pinned down. 
If it turns out to be a real bug I will send it as its own patch.

Also my bad, I realised I had not replied to this thread addressing 
some things you have said, after submitting the v3 patch.

Wei Jie