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

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

Pre-existing issues:
- [High] Lockless access to sibling device pointers in wacom_mode_change_work() leads to concurrent double-free and Use-After-Free.
- [High] wacom_remove_shared_data() calls kref_put() outside of the list lock, creating a race condition that leads to refcount corruption and Use-After-Free.
- [High] The patch protects touch_input with RCU but fails to protect shared->pen and shared->touch, leaving a Use-After-Free vulnerability in IRQ paths.
--

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

commit db13f6a5a689d086da00c273947930e39aedbb61
Author: Lee Jones [off-list ref]

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

This patch attempts to fix a TOCTOU Use-After-Free race condition by
introducing RCU protection for the touch_input pointer in the wacom_shared
struct, wrapping readers in RCU guards, updating writers, and calling
synchronize_rcu().

[Severity: High]
This is a pre-existing issue, but does shared->pen (and shared->touch) need
the same RCU protection that is being added here for touch_input?

For instance, in wacom_bamboo_pad_pen_event(), shared->pen is accessed
locklessly in an interrupt handler:

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

If the Pen sibling device is disconnected concurrently,
wacom_remove_shared_data() clears shared->pen here. Could a preempted or
concurrent IRQ read the non-NULL shared->pen pointer before it is cleared,
and pass the dangling pointer to hid_input_report() once the Pen device is
unbound and freed?

Additionally, in wacom_mode_change_work(), there is lockless access to
these sibling device pointers:

drivers/hid/wacom_sys.c:wacom_mode_change_work() {
    ...
    wacom1 = hid_get_drvdata(shared->pen);
    wacom_release_resources(wacom1);
    hid_hw_stop(wacom1->hdev);
    ...
}

Could the asynchronous worker on a Touch device read shared->pen concurrently
with the Pen device being unbound, leading to a concurrent double-free or
Use-After-Free of the device memory?

+
+		synchronize_rcu();
 
 		kref_put(&data->kref, wacom_release_shared_data);

[Severity: High]
This isn't a bug introduced by this patch, but since kref_put() is called
here outside of the wacom_udev_list_lock, can this create a race condition
that leads to refcount corruption?

If the refcount drops to 0, it calls the release function which waits for
the lock to remove the object from wacom_udev_list. Concurrently, couldn't a
probe on another device acquire the lock and iterate the list:

drivers/hid/wacom_sys.c:wacom_get_hdev_data() {
    ...
    kref_get(&data->kref);
    return data;
    ...
}

Could this result in incrementing a 0-refcount object, causing a
Use-After-Free when the original thread proceeds to unlink and free the
structure?

 		wacom_wac->shared = NULL;
 	}
 }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716090726.2711293-1-lee@kernel.org?part=2