[PATCH 1/1] HID: wacom: serialize mode changes with device removal
flat view
COLD20d
From: Jinmo Yang <hidden>
Date: 2026-10-04 11:15:35
Also in:
lkml, stable
Subsystem:
hid core layer, hid wacom driver, the rest · Maintainers:
Jiri Kosina, Benjamin Tissoires, Ping Cheng, Jason Gerecke, Linus Torvalds
wacom_mode_change_work() takes both siblings' struct wacom out of
wacom_shared and tears down and rebuilds them, holding no reference to
either. wacom_remove() cancels only the work owned by the device being
removed, so a worker owned by the other sibling keeps using this device
after its remove callback returns, and hid_device_remove() then frees
struct wacom along with the driver's devres group.
Reproduced under KASAN on hid.git master (fe2ec83746e5) with a uhid
reproducer that binds a sibling pair, sends a mode-change report and then
closes one sibling's descriptor, 10 boots out of 10:
BUG: KASAN: slab-use-after-free in wacom_parse_and_register+0x4fa8/0x58e0
Read of size 8 at addr ffff88800be431f8 by task kworker/0:2/75
Workqueue: events wacom_mode_change_work
wacom_parse_and_register+0x4fa8/0x58e0
wacom_mode_change_work+0x333/0x7e0
process_one_work+0xa3b/0x19b0
Allocated by task 11: devm_kmalloc <- wacom_probe
Freed by task 82: devres_release_group <- hid_device_remove,
under uhid_char_release <- __x64_sys_close
The same object is also written: wacom_feature_mapping() stores
features->touch_max into it, four bytes at a fixed offset, with the value
taken from a feature report. Closing one sibling's /dev/uhid descriptor is
all it takes, with no privilege, and a reference on the hid_device would
not help: struct wacom is devm_kzalloc() on &hdev->dev.
Serialize mode-change workers with wacom_remove() using a driver-wide
mutex, cancelling the device's own work first so that a worker blocked on
the mutex cannot deadlock removal. wacom_add_shared_data() registers its
devres action inside the group wacom_parse_and_register() opens, so
wacom_remove() clears this device's shared pen or touch pointer under the
mutex and no later worker can pick it up.
The worker also snapshots both shared pointers and returns on a NULL
wacom_wac.shared, because releasing the first sibling's devres can drop
the last reference to the shared data.
Fixes: 4082da80f46a ("HID: wacom: generic: add mode change touch key")
Cc: stable@vger.kernel.org
Signed-off-by: Jinmo Yang <redacted>
---
Applies to hid.git master (fe2ec83746e5). It does not apply to for-next;
see the cover letter.
drivers/hid/wacom_sys.c | 36 +++++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 40770affdbde..ecd7b38b1418 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c@@ -764,6 +764,7 @@ struct wacom_hdev_data { static LIST_HEAD(wacom_udev_list); static DEFINE_MUTEX(wacom_udev_list_lock); +static DEFINE_MUTEX(wacom_mode_change_lock); static bool wacom_are_sibling(struct hid_device *hdev, struct hid_device *sibling)
@@ -2783,22 +2784,32 @@ static void wacom_remote_work(struct work_struct *work) static void wacom_mode_change_work(struct work_struct *work) { struct wacom *wacom = container_of(work, struct wacom, mode_change_work); - struct wacom_shared *shared = wacom->wacom_wac.shared; + struct wacom_shared *shared; + struct hid_device *pen; + struct hid_device *touch; struct wacom *wacom1 = NULL; struct wacom *wacom2 = NULL; - bool is_direct = wacom->wacom_wac.is_direct_mode; + bool is_direct; int error = 0; - if (shared->pen) { - wacom1 = hid_get_drvdata(shared->pen); + mutex_lock(&wacom_mode_change_lock); + shared = wacom->wacom_wac.shared; + if (!shared) + goto out; + pen = shared->pen; + touch = shared->touch; + is_direct = wacom->wacom_wac.is_direct_mode; + + if (pen) { + wacom1 = hid_get_drvdata(pen); wacom_release_resources(wacom1); hid_hw_stop(wacom1->hdev); wacom1->wacom_wac.has_mode_change = true; wacom1->wacom_wac.is_direct_mode = is_direct; } - if (shared->touch) { - wacom2 = hid_get_drvdata(shared->touch); + if (touch) { + wacom2 = hid_get_drvdata(touch); wacom_release_resources(wacom2); hid_hw_stop(wacom2->hdev); wacom2->wacom_wac.has_mode_change = true;
@@ -2808,16 +2819,17 @@ static void wacom_mode_change_work(struct work_struct *work) if (wacom1) { error = wacom_parse_and_register(wacom1, false); if (error) - return; + goto out; } if (wacom2) { error = wacom_parse_and_register(wacom2, false); if (error) - return; + goto out; } - return; +out: + mutex_unlock(&wacom_mode_change_lock); } static int wacom_probe(struct hid_device *hdev,
@@ -2916,6 +2928,10 @@ static void wacom_remove(struct hid_device *hdev) cancel_work_sync(&wacom->battery_work); cancel_work_sync(&wacom->remote_work); cancel_work_sync(&wacom->mode_change_work); + + /* A sibling's mode-change work can also access this device. */ + mutex_lock(&wacom_mode_change_lock); + timer_delete_sync(&wacom->idleprox_timer); if (hdev->bus == BUS_BLUETOOTH) device_remove_file(&hdev->dev, &dev_attr_speed);
@@ -2925,6 +2941,8 @@ static void wacom_remove(struct hid_device *hdev) if (wacom->wacom_wac.features.type != REMOTE) wacom_release_resources(wacom); + + mutex_unlock(&wacom_mode_change_lock); } static int wacom_resume(struct hid_device *hdev)
--
2.53.0