[PATCH] HID: hidraw: fix use-after-free and double free on disconnect

[PATCH] HID: hidraw: fix use-after-free and double free on disconnect

[Jinmo Yang]

BUG: KASAN: slab-use-after-free in _raw_spin_lock_irqsave+0xf8/0x1c8
Write of size 4 at addr ffff0000ce4ffb38 by task wacom_uaf/261

Call trace:
 _raw_spin_lock_irqsave+0xf8/0x1c8
 hidraw_report_event+0x64/0x2a8
 hid_report_raw_event+0x488/0x1228
 __hid_input_report+0x418/0x5b8
 hid_safe_input_report+0x68/0x90
 uhid_char_write+0xb04/0xfb8
 vfs_write+0x2e4/0xac0

Allocated by task 131:
 hidraw_connect+0x54/0x390
 hid_connect+0xb80/0x13d8
 hid_hw_start+0xc4/0x150
 wacom_parse_and_register+0x2ae4/0x4d40
 wacom_probe+0x724/0xb18

Freed by task 68:
 kfree+0x264/0x4c8
 drop_ref+0x268/0x340
 hidraw_disconnect+0x60/0x88
 hid_disconnect+0x17c/0x1c8
 hid_hw_stop+0x70/0x100
 wacom_mode_change_work+0x11c/0x6b0

The buggy address belongs to the object at ffff0000ce4ffb00
 which belongs to the cache kmalloc-96 of size 96
The buggy address is located 56 bytes inside of
 freed 96-byte region [ffff0000ce4ffb00, ffff0000ce4ffb60)

Reproduced on mainline 7.2.0-g66498c75b4f8 (arm64, KASAN) through
/dev/uhid writes alone.  Offset 56 is struct hidraw::list_lock.

hidraw_report_event() dereferences hid->hidraw with nothing keeping the
object alive, and hidraw_disconnect() frees it without clearing the
pointer:

	void hidraw_disconnect(struct hid_device *hid)
	{
		struct hidraw *hidraw = hid->hidraw;

		down_write(&minors_rwsem);
		drop_ref(hidraw, 1);		/* kfree() when !open */
		up_write(&minors_rwsem);
	}			/* hid->hidraw left dangling */

hid_disconnect() also clears the HID_CLAIMED_HIDRAW bit that guards the
call only after the free, so a report that passed the guard walks into
freed memory and the first thing it does there is take a spinlock.

driver_input_lock normally keeps the two apart -- hid_device_remove()
holds it across the callback and __hid_input_report() takes it -- but
hid_hw_stop() carries no such requirement and drivers call it from
workqueue context holding nothing, as wacom_mode_change_work() does.
minors_rwsem cannot be used on the reader side: hidraw_report_event()
also runs from softirq on real transports.

The same function also takes the pointer outside the lock that frees
it.  hid_disconnect() tests hdev->claimed & HID_CLAIMED_HIDRAW with
nothing excluding a second caller and clears claimed only at the end,
so two overlapping teardowns can both reach drop_ref() with the same
pointer.  I have one captured double free of this object with that
shape:

  BUG kmalloc-96: Object already free
   drop_ref+0x70/0x108
   hidraw_disconnect+0x3c/0x60
   hid_disconnect+0x88/0xb0
   hid_hw_stop+0x30/0x70
   wacom_remove+0x30/0x100

I could not isolate that branch to reproduce it on demand -- three
other faults on the same wacom teardown path fire first -- so I am not
claiming this trace proves that window.  Taking the pointer under
minors_rwsem is correct regardless of how often it is hit.

Publish the object through RCU and claim it under the lock.
hidraw_connect() stores the pointer with a release, so the
initialisation is visible to a reader that picks it up without holding
anything; readers take the RCU read side and load the pointer once;
hidraw_disconnect() now reads it under minors_rwsem, returns if another
teardown already took it, and detaches it before dropping the last
reference; drop_ref() hands the object to kfree_rcu(), so a reader that
already loaded the pointer is covered by the grace period.

Same victim object and same faulting function as the bot-reported window
addressed by

  https://lore.kernel.org/linux-input/20260628005846.31248-1-alhouseenyousef@gmail.com/
  ("HID: synchronize input before cleaning up a failed probe")

but a different window: that patch re-acquires driver_input_lock in
__hid_device_probe()'s failure path and leaves hid_disconnect() alone.

Fixes: 86166b7bcda0 ("HID: add hidraw interface")
Cc: stable@vger.kernel.org
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
Validated at runtime with a test-only patch that widens the window
between the pointer load and the spin_lock (mdelay(200) in
hidraw_report_event), so the report race becomes deterministic instead
of a few instructions wide.  arm64 QEMU/KVM guests, same reproducer,
4 vCPU each:

  kernel                            exposure        KASAN   signature
  ----------------------------------------------------------------------
  baseline + widening               4700 binds/180s   1     yes, t+9.07s
  WRITE_ONCE + NULL guard only      4656 binds/180s   1     yes
  RCU + widening                    5718 binds/600s   0     no
  RCU, no widening                  7070 binds/180s   0     no

The second row is worth stating: clearing the pointer and adding a NULL
check, without RCU, does not close this.  A reader that has already
loaded the pointer still walks the object after kfree().  That is why
this patch uses a grace period rather than just a NULL guard.

Those rounds cover the report race only.  I also tried to reproduce the
disconnect-vs-disconnect window on its own, on the KASAN=n,
CONFIG_SLUB_DEBUG=y tree the captured double free came from, with a
reproducer that attaches wacom pen and touch as siblings and races
their teardown: 24 boots, drop_ref() never reached.  In the four of
those boots that also tracked the in-flight hid_device pointers, no two
teardowns of the same device were ever inside hidraw_disconnect()
together -- teardowns of the two sibling devices did overlap there, so
the instrumentation was live.  The teardown race itself reproduces
every boot, but three other faults on that path -- a wacom timer on a
freed object, a NULL deref in hid_hw_stop() and a usercopy fault in
uhid_char_read() -- end the guest before the hidraw branch is reached,
and serialising the two hid_hw_stop() calls closes the path altogether.
So that hunk rests on the captured trace plus the lock discipline, not
on a round of its own, and I would rather say so than overstate it.

On the accessors: hid_device::hidraw is a void * with no __rcu
annotation, and four drivers outside hidraw.c dereference it directly
(hid-cp2112.c, hid-u2fzero.c, hid-led.c and hid-core.c), all from
probe/connect paths where the pointer is stable.  Annotating the field
would let this use rcu_dereference()/rcu_assign_pointer() but would
cascade into those four, so the plain accessors are used with the same
ordering rcu_assign_pointer() would give: smp_store_release() is what it
expands to for a non-NULL store, and WRITE_ONCE() is what it expands to
for a constant NULL.  I am happy to do the __rcu conversion as a
separate patch if you would rather have the annotated form.

Not attempted here: clearing hdev->claimed before the *_disconnect()
calls in hid_disconnect().  hid_report_raw_event() has

	if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield)
		hid_process_report(hid, report, cdata, interrupt);

so zeroing claimed makes that condition true and opens hid_process_report()
during teardown instead.
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 9129fabed181..40c4709d5e98 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -354,7 +354,7 @@ static void drop_ref(struct hidraw *hidraw, int exists_bit)
 	if (!hidraw->open) {
 		if (!hidraw->exist) {
 			hidraw_table[hidraw->minor] = NULL;
-			kfree(hidraw);
+			kfree_rcu(hidraw, rcu);
 		} else {
 			/* close device for last reader */
 			hid_hw_close(hidraw->hid);
@@ -569,11 +569,24 @@ static const struct file_operations hidraw_ops = {
 
 int hidraw_report_event(struct hid_device *hid, u8 *data, int len)
 {
-	struct hidraw *dev = hid->hidraw;
 	struct hidraw_list *list;
+	struct hidraw *dev;
 	int ret = 0;
 	unsigned long flags;
 
+	/*
+	 * The object is freed by hidraw_disconnect() with nothing excluding
+	 * this path: hid_hw_stop() carries no quiesce requirement and drivers
+	 * call it from workqueue context.  Hold the RCU read side so the
+	 * object cannot go away between the load and the unlock.
+	 */
+	rcu_read_lock();
+	dev = READ_ONCE(hid->hidraw);
+	if (!dev) {
+		rcu_read_unlock();
+		return 0;
+	}
+
 	spin_lock_irqsave(&dev->list_lock, flags);
 	list_for_each_entry(list, &dev->list, node) {
 		int new_head = (list->head + 1) & (HIDRAW_BUFFER_SIZE - 1);
@@ -592,6 +605,7 @@ int hidraw_report_event(struct hid_device *hid, u8 *data, int len)
 	spin_unlock_irqrestore(&dev->list_lock, flags);
 
 	wake_up_interruptible(&dev->wait);
+	rcu_read_unlock();
 	return ret;
 }
 EXPORT_SYMBOL_GPL(hidraw_report_event);
@@ -644,7 +658,11 @@ int hidraw_connect(struct hid_device *hid)
 	dev->minor = minor;
 
 	dev->exist = 1;
-	hid->hidraw = dev;
+	/*
+	 * Publish with a release so the initialisation above is visible to
+	 * an RCU reader that picks the pointer up without taking any lock.
+	 */
+	smp_store_release(&hid->hidraw, dev);
 
 	up_write(&minors_rwsem);
 out:
@@ -655,10 +673,25 @@ EXPORT_SYMBOL_GPL(hidraw_connect);
 
 void hidraw_disconnect(struct hid_device *hid)
 {
-	struct hidraw *hidraw = hid->hidraw;
+	struct hidraw *hidraw;
 
 	down_write(&minors_rwsem);
 
+	/*
+	 * Claim the object under the lock and detach it before dropping the
+	 * last reference.  Taking it outside lets two concurrent teardowns
+	 * capture the same pointer and both call drop_ref() on it.  Readers
+	 * that have not loaded the pointer yet now see NULL; readers that
+	 * already did are kept safe by the RCU grace period taken by
+	 * kfree_rcu() in drop_ref().
+	 */
+	hidraw = hid->hidraw;
+	if (!hidraw) {
+		up_write(&minors_rwsem);
+		return;
+	}
+
+	WRITE_ONCE(hid->hidraw, NULL);
 	drop_ref(hidraw, 1);
 
 	up_write(&minors_rwsem);
diff --git a/include/linux/hidraw.h b/include/linux/hidraw.h
index 18fd30a288de..87c3beff7a36 100644
--- a/include/linux/hidraw.h
+++ b/include/linux/hidraw.h
@@ -17,6 +17,7 @@ struct hidraw {
 	struct device *dev;
 	spinlock_t list_lock;
 	struct list_head list;
+	struct rcu_head rcu;
 };
 
 struct hidraw_report {

Re: [PATCH] HID: hidraw: fix use-after-free and double free on disconnect

[Chad Aldreda]

On Sun, 11 Oct 2026, Jinmo Yang wrote:
> hidraw_report_event() dereferences hid->hidraw with nothing keeping the
> object alive, and hidraw_disconnect() frees it without clearing the
> pointer:

Tested on mainline d48723711bae (v7.4-rc1-672), on x86_64 in QEMU (4
vCPUs) with KASAN, lockdep, DEBUG_LIST and kmemleak, and without a
window-widening patch. A test program creates two uhid devices with the
Wacom vendor ID that bind hid-wacom as generic pen and touch siblings.
The pen device's descriptor has the WACOM_HID_WD_MODE_CHANGE usage, so a
report that toggles it schedules wacom_mode_change_work(), which stops
and restarts both devices.

Without the patch, toggling that bit about every 2 ms for 20 s (about
9,400 reports, nothing else sent) gave slab-use-after-free reports
reached from hidraw_report_event() in every run (5 of 5; 31 to 61 KASAN
reports per run, which looks like 2 to 4 hits of about 15 reports each),
on an object freed by hidraw_disconnect() <- hid_disconnect() <-
hid_hw_stop() <- wacom_mode_change_work(). The free path matches your
trace, and it reproduces without any window-widening delay.

With the patch, 11 runs of the same (220 s in all) had no KASAN, lockdep
or DEBUG_LIST reports. kmemleak listed unreferenced objects after 2 of
those 11 runs, and after 3 of the 4 unpatched runs that got as far as
the scan; I have not looked at what they are. About 18 million pen and
touch reports over 20 s without any mode changes gave no KASAN, lockdep
or DEBUG_LIST reports with or without the patch.

Sending pen and touch reports while the mode is toggling oopses the
kernel within seconds with or without the patch. In every such run the
first fault is the same NULL pointer dereference in wacom_wac_event(),
called from wacom_wac_report(), and the later reports are on a tainted
kernel. That is in hid-wacom, in a function your v4 NULL-input series
does not change, and after it those runs say nothing about hidraw.

Not tested: the double-free hunk (I never saw two teardowns of the same
device overlap), the NULL check on its own without the RCU change,
reading an open hidraw node during teardown (attempted, but the
hid-wacom fault above comes first), transports that deliver reports from
interrupt context, real hardware, arm64, and module unload (everything
was built in).

The harness and this report were produced with an AI assistant (Claude
Opus 5.5), which also did the earlier runs, including the unpatched
comparison. I ran the final version on the patched kernel myself and got
the same results. The harness is available on request.

Tested-by: Chad Aldreda <ca@modelmiser.com>