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

Wei Jie LAW posted 1 patch 4 weeks, 1 day ago
There is a newer version of this series
drivers/hid/wacom_sys.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH v3] HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce()
Posted by Wei Jie LAW 4 weeks, 1 day ago
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()
Posted by Jason Gerecke 3 weeks, 2 days ago
On Thu, Aug 27, 2026 at 8:33 PM Wei Jie LAW <98lawweijie@gmail.com> wrote:
>
> 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:
>

Thanks for the v3!

I'm still finding the above paragraph kinda hard to understand. What
do you think of something like the following instead?

* * * *

The 'wacom_wac_pen_serial_enforce' function may calculate and pass an
invalid offset to 'hid_field_extract', resulting in memory reads at
incorrect addresses--possibly beyond the end of the report. If a field
in the HID descriptor lists more usages than its Report Count actually
reserves space for, the function's inner 'j' will walk past the end of
the field:

* * * *

Other than the above language issue:
Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com

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....

>         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()
Posted by Wei Jie LAW 3 weeks, 1 day ago
On Thu, Sep 3, 2026 at 3:36 PM Jason Gerecke <killertofu@gmail.com> wrote:
>
> I'm still finding the above paragraph kinda hard to understand. What
> do you think of something like the following instead?

Your version is definitely easier to understand. I'm good with it!

> Other than the above language issue:
> Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com

Thanks. Should I submit a v4 patch with your "Reviewed-by" and the
updated language, or is leaving it as-is (here) just fine?

Regards,
Wei Jie
[PATCH v4] HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce()
Posted by Wei Jie LAW 2 weeks, 3 days ago
The 'wacom_wac_pen_serial_enforce()' function may calculate and pass an
invalid offset to hid_field_extract(), resulting in memory reads at
incorrect addresses -- possibly beyond the end of the report.  If a
field in the HID descriptor lists more usages than its Report Count
actually reserves space for, the function's inner 'j' will walk past
the end of the field:

	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);

A descriptor 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>
Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
---

Changes in v4, per Jason Gerecke's review of v3:

 - reworded the opening paragraph using Jason's suggested text; 

 - picked up Jason's Reviewed-by

 - no functional change: the diff is byte-for-byte the v3 diff

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.

v3: https://lore.kernel.org/linux-input/20260828033323.82958-1-98lawweijie@gmail.com/
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 v4] HID: wacom: fix OOB read in wacom_wac_pen_serial_enforce()
Posted by Jiri Kosina 2 weeks, 1 day ago
On Wed, 9 Sep 2026, Wei Jie LAW wrote:

> The 'wacom_wac_pen_serial_enforce()' function may calculate and pass an
> invalid offset to hid_field_extract(), resulting in memory reads at
> incorrect addresses -- possibly beyond the end of the report.  If a
> field in the HID descriptor lists more usages than its Report Count
> actually reserves space for, the function's inner 'j' will walk past
> the end of the field:
> 
> 	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);
> 
> A descriptor 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>
> Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs