Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 0/1] HID: wacom: fix slab-out-of-bounds write in kfifo_copy_in
@ 2026-05-24 13:52 Jinmo Yang
  2026-05-24 13:52 ` [PATCH 1/1] HID: wacom: validate report size before kfifo insert Jinmo Yang
  0 siblings, 1 reply; 5+ messages in thread
From: Jinmo Yang @ 2026-05-24 13:52 UTC (permalink / raw)
  To: Jason Gerecke, Ping Cheng
  Cc: Jinmo Yang, Jiri Kosina, Benjamin Tissoires, linux-input,
	linux-kernel, stable

Hi,

I found the following slab-out-of-bounds write in the wacom HID driver
while fuzzing with syzkaller on v7.1.0-rc4-next-20260522:

  BUG: KASAN: slab-out-of-bounds in kfifo_copy_in+0xf3/0x130 lib/kfifo.c:106
  Write of size 3842 at addr ffff888009179000 by task syz.3.9362/61135

  CPU: 1 UID: 0 PID: 61135 Comm: syz.3.9362 Not tainted 7.1.0-rc4-next-20260522-dirty #3 PREEMPT(lazy)
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
  Call Trace:
   <TASK>
   __dump_stack lib/dump_stack.c:94 [inline]
   dump_stack_lvl+0x97/0xe0 lib/dump_stack.c:120
   print_address_description mm/kasan/report.c:378 [inline]
   print_report+0x157/0x4c9 mm/kasan/report.c:482
   kasan_report+0xce/0x100 mm/kasan/report.c:595
   check_region_inline mm/kasan/generic.c:186 [inline]
   kasan_check_range+0x10f/0x1e0 mm/kasan/generic.c:200
   __asan_memcpy+0x3c/0x60 mm/kasan/shadow.c:106
   kfifo_copy_in+0xf3/0x130 lib/kfifo.c:106
   __kfifo_in_r lib/kfifo.c:442 [inline]
   __kfifo_in_r+0x1b2/0x230 lib/kfifo.c:434
   wacom_wac_queue_insert drivers/hid/wacom_sys.c:65 [inline]
   wacom_wac_pen_serial_enforce drivers/hid/wacom_sys.c:165 [inline]
   wacom_raw_event+0x900/0xa90 drivers/hid/wacom_sys.c:179
   __hid_input_report.constprop.0+0x39a/0x4d0 drivers/hid/hid-core.c:2161
   uhid_dev_input2 drivers/hid/uhid.c:618 [inline]
   uhid_char_write+0xa8a/0xfa0 drivers/hid/uhid.c:776
   vfs_write+0x2c0/0xe40 fs/read_write.c:686
   ksys_write+0x1f8/0x250 fs/read_write.c:740
   do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
   do_syscall_64+0xee/0x590 arch/x86/entry/syscall_64.c:94
   entry_SYSCALL_64_after_hwframe+0x77/0x7f

  Allocated by task 4174:
   kasan_save_stack+0x30/0x50 mm/kasan/common.c:57
   kasan_save_track+0x14/0x30 mm/kasan/common.c:78
   poison_kmalloc_redzone mm/kasan/common.c:398 [inline]
   __kasan_kmalloc+0x8f/0xa0 mm/kasan/common.c:415
   kasan_kmalloc include/linux/kasan.h:263 [inline]
   __do_kmalloc_node mm/slub.c:5309 [inline]
   __kmalloc_node_noprof+0x19a/0x4e0 mm/slub.c:5315
   _kmalloc_array_node_noprof include/linux/slab.h:1269 [inline]
   __kfifo_alloc_node+0x11e/0x260 lib/kfifo.c:44
   __kfifo_alloc include/linux/kfifo.h:932 [inline]
   wacom_devm_kfifo_alloc drivers/hid/wacom_sys.c:1315 [inline]
   wacom_parse_and_register+0x2b4/0x5640 drivers/hid/wacom_sys.c:2381
   wacom_probe+0x8d5/0xc40 drivers/hid/wacom_sys.c:2880

  The buggy address belongs to the object at ffff888009179000
   which belongs to the cache kmalloc-256 of size 256
  The buggy address is located 0 bytes inside of
   allocated 256-byte region [ffff888009179000, ffff888009179100)

  Memory state around the buggy address:
   ffff888009179000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
   ffff888009179080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  >ffff888009179100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
                     ^
   ffff888009179180: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
   ffff888009179200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

This is a regression from commit 5e013ad20689 ("HID: wacom: Remove
static WACOM_PKGLEN_MAX limit"), first present in v6.15-rc1. Before
that commit, wacom_raw_event() rejected reports exceeding
WACOM_PKGLEN_MAX (361 bytes) and the kfifo was sized at 512 bytes
(361 rounded up). After the commit, the size cap was removed and the
kfifo is dynamically sized as min(PAGE_SIZE, 10 * pktlen), which can
be as small as 256 bytes.

wacom_wac_queue_insert() passes the report size directly to kfifo_in()
without validating that it fits. When a UHID_INPUT2 event delivers a
report up to 4096 bytes (UHID_DATA_MAX), kfifo_copy_in() writes up to
3840 bytes past the end of the kmalloc-256 slab object.

The fix adds a bounds check in wacom_wac_queue_insert() to reject
reports that exceed the kfifo capacity.

Thanks,
Jinmo

Jinmo Yang (1):
  HID: wacom: validate report size before kfifo insert

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

-- 
2.53.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] HID: wacom: validate report size before kfifo insert
  2026-05-24 13:52 [PATCH 0/1] HID: wacom: fix slab-out-of-bounds write in kfifo_copy_in Jinmo Yang
@ 2026-05-24 13:52 ` Jinmo Yang
  2026-05-24 14:31   ` sashiko-bot
  2026-05-27 19:47   ` Dmitry Torokhov
  0 siblings, 2 replies; 5+ messages in thread
From: Jinmo Yang @ 2026-05-24 13:52 UTC (permalink / raw)
  To: Jason Gerecke, Ping Cheng
  Cc: Jinmo Yang, Jiri Kosina, Benjamin Tissoires, linux-input,
	linux-kernel, stable

wacom_wac_queue_insert() passes the report size directly to kfifo_in()
without checking whether the report fits in the kfifo buffer.

Since commit 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX
limit"), the kfifo is sized dynamically as min(PAGE_SIZE, 10 * pktlen),
which can be as small as 256 bytes. However, reports received via
UHID_INPUT2 can be up to UHID_DATA_MAX (4096) bytes. When such an
oversized report reaches wacom_wac_queue_insert(), the existing
kfifo_avail() loop cannot make room for a record larger than the total
buffer, causing kfifo_copy_in() to memcpy up to 3840 bytes past the
slab allocation.

Add a size check at the top of wacom_wac_queue_insert() to reject
reports that exceed the kfifo capacity.

Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
Cc: stable@vger.kernel.org
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
 drivers/hid/wacom_sys.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a32320b..cc82c6f 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -54,6 +54,12 @@ static void wacom_wac_queue_insert(struct hid_device *hdev,
 {
 	bool warned = false;
 
+	if (size > kfifo_size(fifo)) {
+		hid_warn(hdev, "%s: report too large (%d > %u) for kfifo\n",
+			 __func__, size, kfifo_size(fifo));
+		return;
+	}
+
 	while (kfifo_avail(fifo) < size) {
 		if (!warned)
 			hid_warn(hdev, "%s: kfifo has filled, starting to drop events\n", __func__);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] HID: wacom: validate report size before kfifo insert
  2026-05-24 13:52 ` [PATCH 1/1] HID: wacom: validate report size before kfifo insert Jinmo Yang
@ 2026-05-24 14:31   ` sashiko-bot
  2026-05-27 19:47   ` Dmitry Torokhov
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-05-24 14:31 UTC (permalink / raw)
  To: Jinmo Yang; +Cc: linux-input, dmitry.torokhov

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

New issues:
- [Critical] Incomplete vulnerability fix / off-by-two error allowing kfifo state corruption and massive out-of-bounds slab read.

Pre-existing issues:
- [High] Sleeping in atomic context due to kzalloc(GFP_KERNEL) in wacom_wac_queue_flush().
--

commit 077dd88850a2e1126c613aafa226a5f028a98bb9
Author: Jinmo Yang <jinmo44.yang@gmail.com>

HID: wacom: validate report size before kfifo insert

This commit adds a check in wacom_wac_queue_insert() to reject reports
that exceed the total kfifo capacity. This aims to prevent oversized
reports from causing out-of-bounds writes during kfifo insertion.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index a32320b351e3e..cc82c6f36901a 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -54,6 +54,12 @@ static void wacom_wac_queue_insert(struct hid_device *hdev,
>  {
>  	bool warned = false;
>  
> +	if (size > kfifo_size(fifo)) {

[Severity: Critical]
Does this validation correctly account for the record header overhead?

Since the driver uses a record-based kfifo (kfifo_rec_ptr_2), each inserted 
record is prefixed with a 2-byte length header. This means the maximum 
available payload space in an empty kfifo is kfifo_size(fifo) - 2.

If an attacker sends a report with a size exactly equal to kfifo_size(fifo) 
or kfifo_size(fifo) - 1, this check falsely passes.

> +		hid_warn(hdev, "%s: report too large (%d > %u) for kfifo\n",
> +			 __func__, size, kfifo_size(fifo));
> +		return;
> +	}
> +
>  	while (kfifo_avail(fifo) < size) {

If the payload requires more space than the empty kfifo can provide, 
the kfifo_avail() check will be smaller than size, and the loop will 
execute on an empty buffer:

>  		if (!warned)
>  			hid_warn(hdev, "%s: kfifo has filled, starting to drop events\n", __func__);
>  		warned = true;
>  
>  		kfifo_skip(fifo);
>  	}

Could calling kfifo_skip() on an empty kfifo underflow the internal 
pointers and corrupt the ring buffer's state? 

If this allows the subsequent kfifo_in() to write an oversized payload 
and overwrite the 2-byte record header with attacker-controlled data, 
would this lead to a massive out-of-bounds read later when 
wacom_wac_queue_flush() processes the corrupted record?

[Severity: High]
This is a pre-existing issue, but does the related flush function 
wacom_wac_queue_flush() safely allocate memory?

It appears wacom_wac_queue_flush() calls kzalloc() with GFP_KERNEL. 
Since this queue processing path is invoked from wacom_raw_event() 
(which executes in an atomic context from the USB URB completion handler), 
could this GFP_KERNEL allocation sleep and trigger a 'scheduling while 
atomic' panic? Should this use GFP_ATOMIC instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524135203.1996265-2-jinmo44.yang@gmail.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] HID: wacom: validate report size before kfifo insert
  2026-05-24 13:52 ` [PATCH 1/1] HID: wacom: validate report size before kfifo insert Jinmo Yang
  2026-05-24 14:31   ` sashiko-bot
@ 2026-05-27 19:47   ` Dmitry Torokhov
  2026-05-27 21:41     ` Dmitry Torokhov
  1 sibling, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2026-05-27 19:47 UTC (permalink / raw)
  To: Jinmo Yang
  Cc: Jason Gerecke, Ping Cheng, Jiri Kosina, Benjamin Tissoires,
	linux-input, linux-kernel, stable

On Sun, May 24, 2026 at 10:52:03PM +0900, Jinmo Yang wrote:
> wacom_wac_queue_insert() passes the report size directly to kfifo_in()
> without checking whether the report fits in the kfifo buffer.
> 
> Since commit 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX
> limit"), the kfifo is sized dynamically as min(PAGE_SIZE, 10 * pktlen),
> which can be as small as 256 bytes. However, reports received via
> UHID_INPUT2 can be up to UHID_DATA_MAX (4096) bytes. When such an
> oversized report reaches wacom_wac_queue_insert(), the existing
> kfifo_avail() loop cannot make room for a record larger than the total
> buffer, causing kfifo_copy_in() to memcpy up to 3840 bytes past the
> slab allocation.

Does it? Or maybe spins there indefinitely? Also, doesn't
kfifo_copy_in() return 0 if a record it too big and not copy anything?

> 
> Add a size check at the top of wacom_wac_queue_insert() to reject
> reports that exceed the kfifo capacity.
> 
> Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
> ---
>  drivers/hid/wacom_sys.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index a32320b..cc82c6f 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -54,6 +54,12 @@ static void wacom_wac_queue_insert(struct hid_device *hdev,
>  {
>  	bool warned = false;
>  
> +	if (size > kfifo_size(fifo)) {
> +		hid_warn(hdev, "%s: report too large (%d > %u) for kfifo\n",
> +			 __func__, size, kfifo_size(fifo));
> +		return;
> +	}
> +
>  	while (kfifo_avail(fifo) < size) {
>  		if (!warned)
>  			hid_warn(hdev, "%s: kfifo has filled, starting to drop events\n", __func__);

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] HID: wacom: validate report size before kfifo insert
  2026-05-27 19:47   ` Dmitry Torokhov
@ 2026-05-27 21:41     ` Dmitry Torokhov
  0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2026-05-27 21:41 UTC (permalink / raw)
  To: Jinmo Yang
  Cc: Jason Gerecke, Ping Cheng, Jiri Kosina, Benjamin Tissoires,
	linux-input, linux-kernel, stable

On Wed, May 27, 2026 at 12:47:03PM -0700, Dmitry Torokhov wrote:
> On Sun, May 24, 2026 at 10:52:03PM +0900, Jinmo Yang wrote:
> > wacom_wac_queue_insert() passes the report size directly to kfifo_in()
> > without checking whether the report fits in the kfifo buffer.
> > 
> > Since commit 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX
> > limit"), the kfifo is sized dynamically as min(PAGE_SIZE, 10 * pktlen),
> > which can be as small as 256 bytes. However, reports received via
> > UHID_INPUT2 can be up to UHID_DATA_MAX (4096) bytes. When such an
> > oversized report reaches wacom_wac_queue_insert(), the existing
> > kfifo_avail() loop cannot make room for a record larger than the total
> > buffer, causing kfifo_copy_in() to memcpy up to 3840 bytes past the
> > slab allocation.
> 
> Does it? Or maybe spins there indefinitely? Also, doesn't
> kfifo_copy_in() return 0 if a record it too big and not copy anything?

OK, so the root cause is that kfifo_skip() must not be called on an
empty fifo. I think you want the code to look something like this:

static void wacom_wac_queue_insert(struct hid_device *hdev,
				   struct kfifo_rec_ptr_2 *fifo,
				   u8 *raw_data, int size)
{
	bool warned = false;

	while (kfifo_avail(fifo) < size && !kfifo_is_empty(fifo)) {
		if (!warned)
			hid_warn(hdev, "%s: kfifo has filled, starting to drop events\n", __func__);
		warned = true;

		kfifo_skip(fifo);
	}

	if (!kfifo_in(fifo, raw_data, size))
		hid_warn_ratelimited(hdev, "%s: report is too large (%d)\n",
				     __func__, size);
}

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-27 21:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-24 13:52 [PATCH 0/1] HID: wacom: fix slab-out-of-bounds write in kfifo_copy_in Jinmo Yang
2026-05-24 13:52 ` [PATCH 1/1] HID: wacom: validate report size before kfifo insert Jinmo Yang
2026-05-24 14:31   ` sashiko-bot
2026-05-27 19:47   ` Dmitry Torokhov
2026-05-27 21:41     ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox