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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
> Assisted-by: Claude:claude-opus-5
> Assisted-by: GLM:glm-5.3
> Signed-off-by: Wei Jie Law <98lawweijie@xxxxxxxxx>
> ---
> 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@xxxxxxxxx/
>
>  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
>
>