[PATCH] HID: wacom: handle kmemdup failure in Bluetooth IRQ path

[PATCH] HID: wacom: handle kmemdup failure in Bluetooth IRQ path

[Ruoyu Wang]

wacom_intuos_bt_irq() duplicates the input report with kmemdup() and then
uses data[0] to dispatch the report type. If the allocation fails, the
switch statement dereferences a NULL pointer.

Handle allocation failure by dropping the report. This keeps the
interrupt path from dereferencing a NULL buffer while preserving the
existing return convention for ignored or malformed reports.

Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/hid/wacom_wac.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625d..c42af15e7dba0 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1206,6 +1206,9 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
 	int i = 1;
 	unsigned power_raw, battery_capacity, bat_charging, ps_connected;
 
+	if (!data)
+		return 0;
+
 	switch (data[0]) {
 	case 0x04:
 		if (len < 32) {
-- 
2.34.1

Re: [PATCH] HID: wacom: handle kmemdup failure in Bluetooth IRQ path

[Jason Gerecke]

On Fri, Jun 5, 2026 at 9:04 PM Ruoyu Wang <ruoyuw560@gmail.com> wrote:
>
> wacom_intuos_bt_irq() duplicates the input report with kmemdup() and then
> uses data[0] to dispatch the report type. If the allocation fails, the
> switch statement dereferences a NULL pointer.
>
> Handle allocation failure by dropping the report. This keeps the
> interrupt path from dereferencing a NULL buffer while preserving the
> existing return convention for ignored or malformed reports.
>
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
>  drivers/hid/wacom_wac.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625d..c42af15e7dba0 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1206,6 +1206,9 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
>         int i = 1;
>         unsigned power_raw, battery_capacity, bat_charging, ps_connected;
>
> +       if (!data)
> +               return 0;
> +

Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>

Though, I think we should also (or instead?) entertain a change that
does away with the kmemdup entirely... Replacing this kmemdup and the
memcpy in wacom_intuos_bt_process_data with some simple pointer
manipulation should have the same effect with less work.

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

>         switch (data[0]) {
>         case 0x04:
>                 if (len < 32) {
> --
> 2.34.1
>
>

[PATCH v2] HID: wacom: avoid copying Bluetooth input reports

[Ruoyu Wang]

wacom_intuos_bt_irq() duplicates the received Bluetooth report with
kmemdup() so that it can pass 10-byte input report payloads to the
common Intuos parser. The helper then copies each payload back into
wacom->data before calling wacom_intuos_irq().

Avoid the allocation and copy by temporarily pointing wacom->data at the
current 10-byte payload while the common parser runs, then restoring the
original report pointer. The Bluetooth report parser keeps using the
original report buffer for dispatch and battery parsing, while the common
parser sees the same payload bytes as before.

This also removes the unchecked kmemdup() result from the Bluetooth IRQ
path.

Suggested-by: Jason Gerecke <jason.gerecke@wacom.com>
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
Changes in v2:
- Replace the kmemdup()/memcpy() path with temporary wacom->data pointer
  substitution, as suggested by Jason.

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

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625d..a29bf051ada7c 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1192,8 +1192,11 @@ static int int_dist(int x1, int y1, int x2, int y2)
 static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,
 		unsigned char *data)
 {
-	memcpy(wacom->data, data, 10);
+	u8 *saved_data = wacom->data;
+
+	wacom->data = data;
 	wacom_intuos_irq(wacom);
+	wacom->data = saved_data;
 
 	input_sync(wacom->pen_input);
 	if (wacom->pad_input)
@@ -1202,7 +1205,7 @@ static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,
 
 static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
 {
-	u8 *data = kmemdup(wacom->data, len, GFP_KERNEL);
+	u8 *data = wacom->data;
 	int i = 1;
 	unsigned power_raw, battery_capacity, bat_charging, ps_connected;
 
@@ -1242,7 +1245,6 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
 		break;
 	}
 
-	kfree(data);
 	return 0;
 }
 
-- 
2.51.0

Re: [PATCH v2] HID: wacom: avoid copying Bluetooth input reports

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unaligned memory access when parsing Bluetooth input reports.
--

commit 3d01afcf428b29530738d10b1c36a61efcc9ab57
Author: Ruoyu Wang <ruoyuw560@gmail.com>

HID: wacom: avoid copying Bluetooth input reports

This commit avoids duplicating the Bluetooth report with kmemdup() by
temporarily pointing wacom->data at the 10-byte payload while the
common parser runs.

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625d..a29bf051ada7c 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1192,8 +1192,11 @@ static int int_dist(int x1, int y1, int x2, int y2)
>  static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,
>  		unsigned char *data)
>  {
> -	memcpy(wacom->data, data, 10);
> +	u8 *saved_data = wacom->data;
> +
> +	wacom->data = data;

[Severity: High]
Can this lead to unaligned memory accesses on strict-alignment architectures?

In wacom_intuos_bt_irq(), wacom_intuos_bt_process_data() is called with
data + 1 or data + 11. By pointing wacom->data directly to these unaligned
offsets, the common parsers will receive an odd-aligned base pointer.

When wacom_intuos_irq() then calls wacom_intuos_general() or
wacom_intuos_pad(), those functions assume wacom->data is aligned and cast
offsets from it to __be16 *:

drivers/hid/wacom_wac.c:wacom_intuos_general() {
        ...
        x = (be16_to_cpup((__be16 *)&data[2]) << 1) | ((data[9] >> 1) & 1);
        ...
}

If data is odd-aligned, &data[2] will also be unaligned. Calling
be16_to_cpup() on an unaligned pointer will trigger alignment faults on
architectures that do not support hardware unaligned accesses.

>  	wacom_intuos_irq(wacom);
> +	wacom->data = saved_data;
>  
>  	input_sync(wacom->pen_input);
>  	if (wacom->pad_input)

[ ... ]

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

Re: [PATCH v2] HID: wacom: avoid copying Bluetooth input reports

[Jason Gerecke]

On Wed, Jun 17, 2026 at 12:21 AM Ruoyu Wang <ruoyuw560@gmail.com> wrote:
>
> wacom_intuos_bt_irq() duplicates the received Bluetooth report with
> kmemdup() so that it can pass 10-byte input report payloads to the
> common Intuos parser. The helper then copies each payload back into
> wacom->data before calling wacom_intuos_irq().
>
> Avoid the allocation and copy by temporarily pointing wacom->data at the
> current 10-byte payload while the common parser runs, then restoring the
> original report pointer. The Bluetooth report parser keeps using the
> original report buffer for dispatch and battery parsing, while the common
> parser sees the same payload bytes as before.
>
> This also removes the unchecked kmemdup() result from the Bluetooth IRQ
> path.
>
> Suggested-by: Jason Gerecke <jason.gerecke@wacom.com>
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>

Thanks for the update! The sashiko warnings reveal an existing issue
with this function triggering unaligned memory accesses. I don't think
the warnings should block this particular patch, but instead remind us
that some kind of dedicated alignment cleanup patch would be a good
idea.

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

> ---
> Changes in v2:
> - Replace the kmemdup()/memcpy() path with temporary wacom->data pointer
>   substitution, as suggested by Jason.
>
>  drivers/hid/wacom_wac.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625d..a29bf051ada7c 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1192,8 +1192,11 @@ static int int_dist(int x1, int y1, int x2, int y2)
>  static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,
>                 unsigned char *data)
>  {
> -       memcpy(wacom->data, data, 10);
> +       u8 *saved_data = wacom->data;
> +
> +       wacom->data = data;
>         wacom_intuos_irq(wacom);
> +       wacom->data = saved_data;
>
>         input_sync(wacom->pen_input);
>         if (wacom->pad_input)
> @@ -1202,7 +1205,7 @@ static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,
>
>  static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
>  {
> -       u8 *data = kmemdup(wacom->data, len, GFP_KERNEL);
> +       u8 *data = wacom->data;
>         int i = 1;
>         unsigned power_raw, battery_capacity, bat_charging, ps_connected;
>
> @@ -1242,7 +1245,6 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
>                 break;
>         }
>
> -       kfree(data);
>         return 0;
>  }
>
> --
> 2.51.0
>