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