[PATCH] HID: wacom: validate GRAPHIRE_BT report length

[PATCH] HID: wacom: validate GRAPHIRE_BT report length

[Aamir Ahmed]

wacom_graphire_irq() parses GRAPHIRE_BT reports by dereferencing
wacom->data at fixed offsets up to data[7], guarded only by the
data[0] == WACOM_REPORT_PENABLED_BT report-id check. It never consults
the report length, and wacom_wac_irq() does not pass len down for this
device type.

A malformed, malfunctioning or spoofed Bluetooth Graphire (VID 0x056a,
PID 0x81) can send a report shorter than 8 bytes that still passes the
report-id check. The handler then reads past the received report and
forwards the bytes to userspace as ABS_X/ABS_Y/ABS_PRESSURE/ABS_MISC/
ABS_DISTANCE and battery state.

On the Bluetooth transports the backing buffer is large and fixed (hidp
copies into input_buf[HID_MAX_BUFFER_SIZE]; uhid uses UHID_DATA_MAX), so
this reads stale in-buffer bytes rather than overrunning the allocation.
It is the same missing-length-check class already fixed in the sibling
handlers wacom_intuos_bt_irq() (commit 2f1763f62909 ("HID: wacom: fix
out-of-bounds read in wacom_intuos_bt_irq")) and
wacom_intuos_pro2_bt_irq() (commit a8e04f3f894c ("HID: wacom: validate
report length in wacom_intuos_pro2_bt_irq")); the GRAPHIRE_BT handler was
not covered by those changes. The issue was found by LLM-assisted variant
analysis of those two fixes.

Pass the wire length to wacom_graphire_irq() and reject GRAPHIRE_BT
reports shorter than the furthest offset the parser dereferences.

Fixes: 387142bb8fcb ("Input: wacom - handle Graphire BT tablets in wacom.ko")
Assisted-by: LLM:opus-4.8
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
---

Notes (not part of the changelog):
- Compile-tested on x86_64 with CONFIG_HID_WACOM=m; builds with no new
  warnings. No runtime reproducer: per the changelog, on the hidp/uhid
  transports this is a stale-buffer read, so KASAN is not expected to fire.
- Found by LLM-assisted variant analysis of the two sibling fixes cited
  above (see the Assisted-by tag).
 drivers/hid/wacom_wac.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 8feb8027be95..31da8b956607 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -338,7 +338,7 @@ static int wacom_dtus_irq(struct wacom_wac *wacom)
 	}
 }
 
-static int wacom_graphire_irq(struct wacom_wac *wacom)
+static int wacom_graphire_irq(struct wacom_wac *wacom, size_t len)
 {
 	struct wacom_features *features = &wacom->features;
 	unsigned char *data = wacom->data;
@@ -350,6 +350,12 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
 	int retval = 0;
 
 	if (features->type == GRAPHIRE_BT) {
+		if (len < WACOM_PKGLEN_PENABLED) {
+			dev_warn(input->dev.parent,
+				 "Graphire BT report too short: %zu bytes\n",
+				 len);
+			goto exit;
+		}
 		if (data[0] != WACOM_REPORT_PENABLED_BT) {
 			dev_dbg(input->dev.parent,
 				"%s: received unknown report #%d\n", __func__,
@@ -3475,7 +3481,7 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
 	case GRAPHIRE:
 	case GRAPHIRE_BT:
 	case WACOM_MO:
-		sync = wacom_graphire_irq(wacom_wac);
+		sync = wacom_graphire_irq(wacom_wac, len);
 		break;
 
 	case PTU:
-- 
2.53.0.windows.1

Re: [PATCH] HID: wacom: validate GRAPHIRE_BT report length

[sashiko-bot]

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

Pre-existing issues:
- [High] The length check added to wacom_graphire_irq is scoped exclusively to GRAPHIRE_BT devices, leaving other device types (e.g., WACOM_MO, WACOM_G4, GRAPHIRE) and other handlers (e.g., wacom_intuos_irq) vulnerable to identical stale-buffer reads.
- [High] Missing cancellation of asynchronous timers and workqueues in the wacom_probe() error path leads to Use-After-Free.
--

commit 60032e23affbdf87e0c943f82b7987db425fb939
Author: Aamir Ahmed <elb12345@hotmail.co.uk>

HID: wacom: validate GRAPHIRE_BT report length

This commit adds a length check for GRAPHIRE_BT reports in
wacom_graphire_irq() to prevent out-of-bounds reads on short packets.
If the report length is less than expected, it logs a warning and exits.

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 8feb8027be95e..31da8b9566077 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c

[ ... ]

> @@ -350,6 +350,12 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
>  	int retval = 0;
>  
>  	if (features->type == GRAPHIRE_BT) {
> +		if (len < WACOM_PKGLEN_PENABLED) {
> +			dev_warn(input->dev.parent,
> +				 "Graphire BT report too short: %zu bytes\n",
> +				 len);
> +			goto exit;
> +		}

[Severity: High]
This isn't a bug introduced by this patch, but does this bounds check
mistakenly limit itself to GRAPHIRE_BT devices?

If a spoofed or malfunctioning WACOM_MO, WACOM_G4, or GRAPHIRE HID device
sends a report with a length shorter than 9 bytes and a valid report ID,
wacom_graphire_irq() bypasses this new check.

It then proceeds to the fallback branch:

} else if (data[0] != WACOM_REPORT_PENABLED) {

and later accesses data[8] unconditionally for WACOM_MO:

	case WACOM_MO:
		prox = (data[7] & 0xf8) || data[8];

Can this result in an out-of-bounds read of the HID transport buffer,
leaking up to 8 bytes of uninitialized or stale memory into the input
subsystem?

> @@ -3475,7 +3481,7 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
>  	case GRAPHIRE:
>  	case GRAPHIRE_BT:
>  	case WACOM_MO:
> -		sync = wacom_graphire_irq(wacom_wac);
> +		sync = wacom_graphire_irq(wacom_wac, len);
>  		break;
>  
>  	case PTU:

[Severity: High]
This is a pre-existing issue, but is it possible to validate the report
length for other handlers like wacom_intuos_irq()?

Currently, wacom_intuos_irq() does not accept a len argument, which seems
to leave it vulnerable to identical stale-buffer reads.


[Severity: High]
This is a pre-existing issue, but is there a missing cancellation of
asynchronous timers and workqueues in the wacom_probe() error path?

In drivers/hid/wacom_sys.c:wacom_probe(), if a subsequent initialization
step fails after wacom_parse_and_register(), it propagates the failure:

	error = wacom_parse_and_register(wacom, false);
	if (error)
		return error;

Inside wacom_parse_and_register(), the failure path stops the hardware
but fails to cancel pending works like wacom->init_work or
wacom->battery_work:

fail_hw_stop:
	hid_hw_stop(hdev);
fail:
	wacom_release_resources(wacom);
	return error;

Since wacom is allocated via devm_kzalloc, it is immediately freed upon
probe failure.  When the pending work subsequently executes, might it
access the freed wacom struct, causing a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/AS8P251MB00012F559DCFF177B19F767BC8B42@AS8P251MB0001.EURP251.PROD.OUTLOOK.COM?part=1

Re: [PATCH] HID: wacom: validate GRAPHIRE_BT report length

[Aamir Ahmed]

On Sat, Sep 05, 2026 at 10:29:08AM +0000, Sashiko AI wrote:
> [Severity: High]
> This is a pre-existing issue, but is there a missing cancellation of
> asynchronous timers and workqueues in the wacom_probe() error path?

Partly. I checked this and the core of it is real, but narrower than
described.

The only work item that can be pending when wacom_parse_and_register()
fails is init_work. wacom_query_tablet_data() schedules it one second
out before the WL_MONITOR hid_hw_open() call, and if that call fails the
error path does not cancel it. wacom_probe() then returns the error,
__hid_device_probe() releases the devres group and frees the wacom
structure, and wacom_init_work() runs on freed memory a second later.

wireless_work, battery_work, remote_work and the timers cannot be
involved. They are only scheduled from incoming reports, and
__hid_input_report() drops every report while probe() holds
driver_input_lock, which this driver never releases early.

It is also not reachable from user input: it needs one of two devices
and hid_hw_open() failing at that moment, so I would not call it High.
I have sent a fix as a separate patch:

  HID: wacom: Cancel init_work when probe fails after scheduling it

> This isn't a bug introduced by this patch, but does this bounds check
> mistakenly limit itself to GRAPHIRE_BT devices?

The other device types served by wacom_graphire_irq() are USB, where
the transport buffer is at least 64 bytes, so a short report yields
stale bytes from the previous report rather than an out-of-bounds
read. The same applies to wacom_intuos_irq(). Extending the checks
would be reasonable hardening, and I am happy to send that separately
if the maintainers want it, but it is not a memory safety fix.

Thanks,
Aamir