Re: [PATCH 1/1] HID: wacom: serialize mode changes with device removal
From: sashiko-bot@kernel.org
Date: 2026-10-04 11:28:12
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid: wacom: Use-After-Free due to sibling worker restarting hardware during remove
--
commit d345d10fbdd75c5608a4db1f5c93eaec7ccc124e
Author: Jinmo Yang [off-list ref]
Subject: HID: wacom: serialize mode changes with device removal
This commit adds a driver-wide wacom_mode_change_lock mutex to serialize
mode-change workers with wacom_remove(). The goal is to prevent a sibling
device's mode-change worker from accessing a removed device's struct wacom
after it has been freed.
Fixes: 4082da80f46a ("HID: wacom: generic: add mode change touch key")
Cc: stable@vger.kernel.org
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2783,22 +2784,32 @@ static void wacom_remote_work(struct work_struct *work) static void wacom_mode_change_work(struct work_struct *work) {
[ ... ]
quoted hunk ↗ jump to hunk
+ if (pen) { + wacom1 = hid_get_drvdata(pen); wacom_release_resources(wacom1); hid_hw_stop(wacom1->hdev);
[Severity: High] If a sibling worker is running concurrently with wacom_remove(), could this double-stop the hardware of the device being removed? [ ... ]
quoted hunk ↗ jump to hunk
if (wacom1) { error = wacom_parse_and_register(wacom1, false); if (error) - return; + goto out; }
[Severity: High] Could the sibling worker restart the hardware via wacom_parse_and_register() (which calls hid_hw_start()) here, while wacom_remove() is blocked waiting on the wacom_mode_change_lock? [ ... ]
quoted hunk ↗ jump to hunk
@@ -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);
[Severity: High] Since wacom_remove() executes teardown steps like hid_hw_stop() before acquiring the lock, and only cancels its own worker, could a sibling's worker continue running and bypass these teardown steps? [ ... ]
quoted hunk ↗ jump to hunk
if (wacom->wacom_wac.features.type != REMOTE) wacom_release_resources(wacom); + + mutex_unlock(&wacom_mode_change_lock); }
[Severity: High] If the sibling's worker restarted the hardware via wacom_parse_and_register(), wacom_remove() returns here without calling hid_hw_stop() again. Will this leave the device's hidraw nodes active and registered after the driver is unbound, leading to a Use-After-Free of struct wacom when accessed by userspace? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261004111353.118025-1-jinmo44.yang@gmail.com?part=1