Thread (8 messages) 8 messages, 2 authors, 15d ago

Re: [PATCH 2/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad

From: sashiko-bot@kernel.org
Date: 2026-07-07 12:26:57

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

Pre-existing issues:
- [High] Pre-existing use-after-free race between kref_put() in wacom_remove_shared_data() and list traversal in wacom_get_hdev_data().
- [High] Lockless reads of `shared->pen` and `shared->touch` lead to TOCTOU Use-After-Free.
--
--- Patch [2]: [PATCH 2/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad ---
commit 7367a08b48f1d1fc2e7dc8823f95547c3c03eb51
Author: Lee Jones [off-list ref]

HID: wacom: Fix Use-After-Free in wacom_intuos_pad

wacom_intuos_pad() accesses wacom->shared->touch_input locklessly
inside the interrupt handler context.  If the Touch sibling device
is disconnected, wacom_remove_shared_data() clears touch_input
outside any lock, creating a Time-of-Check to Time-of-Use (TOCTOU)
race condition.

Resolve this by introducing RCU protection for the touch_input
pointer.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 6c2a3ddfe366a..c6b37931d02d1 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -875,10 +875,16 @@ static void wacom_remove_shared_data(void *res)
 		data = container_of(wacom_wac->shared, struct wacom_hdev_data,
 				    shared);
 
-		if (wacom_wac->shared->touch == wacom->hdev)
-			wacom_wac->shared->touch = NULL;
-		else if (wacom_wac->shared->pen == wacom->hdev)
-			wacom_wac->shared->pen = NULL;
+		scoped_guard(mutex, &wacom_udev_list_lock) {
+			if (wacom_wac->shared->touch == wacom->hdev) {
+				wacom_wac->shared->touch = NULL;
+				rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
+			} else if (wacom_wac->shared->pen == wacom->hdev) {
+				wacom_wac->shared->pen = NULL;
+			}
+		}
[Severity: High]
This isn't a bug introduced by this patch, but since we are introducing
RCU protection for touch_input, do shared->pen and shared->touch need
similar RCU annotations and read guards?

wacom_mode_change_work() reads both pointers locklessly in preemptible
workqueue context:

drivers/hid/wacom_sys.c:wacom_mode_change_work() {
    ...
    if (shared->pen) {
        wacom1 = hid_get_drvdata(shared->pen);
    ...
    if (shared->touch) {
        wacom2 = hid_get_drvdata(shared->touch);
    ...
}

Similarly, wacom_bamboo_pad_pen_event() dereferences shared->pen without
protection:

drivers/hid/wacom_wac.c:wacom_bamboo_pad_pen_event() {
    ...
    hid_input_report(wacom->shared->pen, HID_INPUT_REPORT, data, ...);
    ...
}

Because these readers run without rcu_read_lock() or guard(rcu), the new
synchronize_rcu() call won't wait for them, potentially leaving them to
dereference freed pointers when the sibling device unbinds. Should pen and
touch also be converted to RCU pointers?
+
+		synchronize_rcu();
 
 		kref_put(&data->kref, wacom_release_shared_data);
[Severity: High]
This is a pre-existing issue, but does this lockless kref_put() create a
race condition with concurrent device probes?

If a concurrent unbind and probe occurs, kref_put() can drop the refcount
to 0 and synchronously call wacom_release_shared_data(). Before the
release function can acquire the wacom_udev_list_lock to remove the
object, a concurrent wacom_add_shared_data() might call
wacom_get_hdev_data().

wacom_get_hdev_data() iterates the list under the lock, finds the still-
linked object, and blindly calls kref_get(&data->kref) on the 0-refcount
object:

drivers/hid/wacom_sys.c:wacom_get_hdev_data() {
    ...
    list_for_each_entry(data, &wacom_udev_list, list) {
        if (strncmp(data->phys, hdev->phys, sizeof(data->phys) - 2) == 0) {
            kref_get(&data->kref);
            return data;
        }
    ...
}

When the lock is dropped, the release function proceeds to kfree() the
object, leaving the prober with a freed pointer. Should this be using
kref_put_mutex() or kref_get_unless_zero() to safely handle the list
removal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707121051.1683937-1-lee@kernel.org?part=2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help