* [PATCH v9 2/5] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration
2026-09-02 13:44 [PATCH v9 1/5] HID: wacom: Use hdev->product in wacom_setup_touch_input_capabilities Lee Jones
@ 2026-09-02 13:44 ` Lee Jones
2026-09-02 14:03 ` sashiko-bot
2026-09-02 13:44 ` [PATCH v9 3/5] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Lee Jones @ 2026-09-02 13:44 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Peter Hutterer, Dmitry Torokhov, linux-input,
linux-kernel
Cc: stable
Input subsystem guidelines require that device capabilities are advertised
before the input device is registered. The Wacom driver was violating
this by advertising the SW_MUTE_DEVICE capability post-registration in
wacom_set_shared_values() (and duplicating it in device-specific setup
cases).
Resolve this by moving the SW_MUTE_DEVICE capability setup to
wacom_setup_touch_input_capabilities() for all touch devices that support
it.
For generic touch devices whose capabilities depend on mute switch
usages parsed from a sibling Pen/Pad interface, defer registration
with -EPROBE_DEFER until the sibling has parsed its descriptors and
initialized shared capabilities.
Cc: stable@vger.kernel.org
Fixes: d2ec58aee8b1 ("HID: wacom: generic: support generic touch switch")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v4 -> v5: New patch used to split out SW_MUTE_DEVICE as per Jason's request
v5 -> v6: Unconditionally advertise SW_MUTE_DEVICE on generic touch devices
v6 -> v7: Only advertise SW_MUTE_DEVICE on composite USB generic touch devices
v7 -> v8: Replace heuristic with probe deferral until sibling Pen/Pad is parsed
Split out 'hdev->product' cleanups into a separate standalone patch
v8 -> v9: Fix TOCTOU race by assigning shared sibling pointers in wacom_set_shared_values()
Support Pad interfaces in sibling deferral logic
drivers/hid/wacom_sys.c | 68 ++++++++++++++++++++++++++++++-----------
drivers/hid/wacom_wac.c | 4 +++
2 files changed, 55 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0eafa483b7f7..026a6be467d3 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -912,14 +912,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
wacom_wac->shared = &data->shared;
retval = devm_add_action_or_reset(&hdev->dev, wacom_remove_shared_data, wacom);
- if (retval)
- return retval;
-
- if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
- wacom_wac->shared->touch = hdev;
- else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
- wacom_wac->shared->pen = hdev;
-
return retval;
}
@@ -2343,10 +2335,7 @@ static void wacom_release_resources(struct wacom *wacom)
static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
{
- if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- wacom_wac->shared->type = wacom_wac->features.type;
- wacom_wac->shared->touch_input = wacom_wac->touch_input;
- }
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
if (wacom_wac->has_mute_touch_switch) {
wacom_wac->shared->has_mute_touch_switch = true;
@@ -2359,14 +2348,54 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
wacom_wac->shared->is_touch_on = true;
}
- if (wacom_wac->shared->has_mute_touch_switch &&
- wacom_wac->shared->touch_input) {
- set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
- input_set_capability(wacom_wac->shared->touch_input, EV_SW,
- SW_MUTE_DEVICE);
+ if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
+ wacom_wac->shared->type = wacom_wac->features.type;
+ wacom_wac->shared->touch_input = wacom_wac->touch_input;
+ wacom_wac->shared->touch = wacom->hdev;
+ } else if (wacom_wac->features.device_type &
+ (WACOM_DEVICETYPE_PEN | WACOM_DEVICETYPE_PAD)) {
+ wacom_wac->shared->pen = wacom->hdev;
}
}
+static bool wacom_sibling_pending(struct wacom *wacom)
+{
+ struct hid_device *hdev = wacom->hdev;
+ const struct wacom_features *features = &wacom->wacom_wac.features;
+ struct usb_host_config *actconfig;
+ int i;
+
+ if (features->type != HID_GENERIC ||
+ !(features->device_type & WACOM_DEVICETYPE_TOUCH))
+ return false;
+
+ if (wacom->wacom_wac.shared &&
+ rcu_access_pointer(wacom->wacom_wac.shared->pen))
+ return false;
+
+ if (!hid_is_usb(hdev) || !wacom->usbdev)
+ return false;
+
+ if (features->oPid != HID_ANY_ID && features->oPid != 0)
+ return true;
+
+ actconfig = wacom->usbdev->actconfig;
+ if (actconfig && actconfig->desc.bNumInterfaces > 1) {
+ for (i = 0; i < actconfig->desc.bNumInterfaces; i++) {
+ struct usb_interface *sibling_intf = actconfig->interface[i];
+
+ if (!sibling_intf || sibling_intf == wacom->intf)
+ continue;
+
+ if (sibling_intf->cur_altsetting->desc.bInterfaceClass ==
+ USB_INTERFACE_CLASS_HID)
+ return true;
+ }
+ }
+
+ return false;
+}
+
static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
{
struct wacom_wac *wacom_wac = &wacom->wacom_wac;
@@ -2444,6 +2473,11 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
if (error)
goto fail;
+ if (wacom_sibling_pending(wacom)) {
+ error = -EPROBE_DEFER;
+ goto fail;
+ }
+
error = wacom_setup_inputs(wacom);
if (error)
goto fail;
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 4436faf2d2bb..a7e3817aa2c4 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3965,6 +3965,10 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
if (features->type == HID_GENERIC) {
hid_dbg(hdev, "generic touch setup\n");
+ if (wacom_wac->shared && wacom_wac->shared->has_mute_touch_switch) {
+ input_set_capability(input_dev, EV_SW, SW_MUTE_DEVICE);
+ wacom_wac->has_mute_touch_switch = true;
+ }
/* setup has already been done */
return 0;
}
--
2.55.0.966.g6673acef38-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v9 2/5] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration
2026-09-02 13:44 ` [PATCH v9 2/5] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration Lee Jones
@ 2026-09-02 14:03 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-02 14:03 UTC (permalink / raw)
To: Lee Jones; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Generic touch devices will enter an infinite -EPROBE_DEFER loop if they share a USB device with a non-Pen/Pad HID interface.
- [High] Standalone generic touch devices with a hardware mute switch will no longer advertise the SW_MUTE_DEVICE capability.
- [Medium] Missing memory barrier / RCU acquire semantics when reading shared sibling state causes a TOCTOU race.
Pre-existing issues:
- [High] Missing cancel_delayed_work_sync in the hid_hw_open error path causes a Use-After-Free.
--
commit bd90131f0064218ea089e2069fff71db08186576
Author: Lee Jones <lee@kernel.org>
Subject: HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration
This commit moves the SW_MUTE_DEVICE capability setup to
wacom_setup_touch_input_capabilities() for touch devices. It also defers
registration for generic touch devices until their sibling Pen/Pad interface
has been parsed and shared capabilities are initialized.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0eafa483b7f75..026a6be467d3d 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
[ ... ]
> @@ -2359,12 +2348,52 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
[ ... ]
> +static bool wacom_sibling_pending(struct wacom *wacom)
> +{
> + struct hid_device *hdev = wacom->hdev;
> + const struct wacom_features *features = &wacom->wacom_wac.features;
> + struct usb_host_config *actconfig;
> + int i;
> +
> + if (features->type != HID_GENERIC ||
> + !(features->device_type & WACOM_DEVICETYPE_TOUCH))
> + return false;
> +
> + if (wacom->wacom_wac.shared &&
> + rcu_access_pointer(wacom->wacom_wac.shared->pen))
[Severity: Medium]
Is there a potential TOCTOU race here due to a missing acquire barrier?
Because rcu_access_pointer() does not provide an acquire memory barrier,
could weakly ordered CPUs reorder the reads, seeing the updated pen pointer
here but a stale false value for the switch flag later when
wacom_setup_touch_input_capabilities() checks shared->has_mute_touch_switch?
> + return false;
> +
> + if (!hid_is_usb(hdev) || !wacom->usbdev)
> + return false;
> +
> + if (features->oPid != HID_ANY_ID && features->oPid != 0)
> + return true;
> +
> + actconfig = wacom->usbdev->actconfig;
> + if (actconfig && actconfig->desc.bNumInterfaces > 1) {
> + for (i = 0; i < actconfig->desc.bNumInterfaces; i++) {
> + struct usb_interface *sibling_intf = actconfig->interface[i];
> +
> + if (!sibling_intf || sibling_intf == wacom->intf)
> + continue;
> +
> + if (sibling_intf->cur_altsetting->desc.bInterfaceClass ==
> + USB_INTERFACE_CLASS_HID)
[Severity: High]
Could this overly broad check lead to an infinite -EPROBE_DEFER loop?
If a generic touch device shares a USB device with a standard HID interface
(like a keyboard or raw vendor HID), the Wacom driver might reject the
standard interface, causing hid-generic to bind to it instead.
Since hid-generic would not populate wacom_wac.shared->pen, this check
would repeatedly observe shared->pen == NULL and a sibling interface with
USB_INTERFACE_CLASS_HID, returning true indefinitely. It seems this could
leave the touch interface permanently trapped in probe deferral.
> + return true;
> + }
> }
> +
> + return false;
> }
[ ... ]
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 4436faf2d2bbd..a7e3817aa2c4f 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -3965,6 +3965,10 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
>
> if (features->type == HID_GENERIC) {
> hid_dbg(hdev, "generic touch setup\n");
> + if (wacom_wac->shared && wacom_wac->shared->has_mute_touch_switch) {
[Severity: High]
For standalone generic touch devices with a hardware mute switch, does this
result in the loss of the SW_MUTE_DEVICE capability?
The local parser sets the wacom_wac->has_mute_touch_switch flag, but this
now only checks the shared flag (wacom_wac->shared->has_mute_touch_switch).
However, wacom_set_shared_values() syncs the local flag to the shared
struct at the very end of wacom_parse_and_register(), which happens
after capabilities are set up. Could this leave the shared flag false
during setup for standalone devices, breaking userspace expectations?
> + input_set_capability(input_dev, EV_SW, SW_MUTE_DEVICE);
> + wacom_wac->has_mute_touch_switch = true;
> + }
> /* setup has already been done */
> return 0;
> }
[ ... ]
[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
in the hid_hw_open() error path of wacom_parse_and_register().
During device probe, wacom_query_tablet_data() schedules wacom->init_work
with a 1-second delay. If the device has WACOM_DEVICETYPE_WL_MONITOR and
the subsequent hid_hw_open() fails:
if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
error = hid_hw_open(hdev);
if (error) {
hid_err(hdev, "hw open failed\n");
goto fail_hw_stop;
}
}
The code jumps to fail_hw_stop without canceling the delayed work. Since
the devres framework frees the wacom structure on probe failure, does this
leave a dangling timer that will attempt to access freed memory when
wacom->init_work runs? Should cancel_delayed_work_sync(&wacom->init_work)
be called in this error path?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902134417.1270552-1-lee@kernel.org?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v9 3/5] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
2026-09-02 13:44 [PATCH v9 1/5] HID: wacom: Use hdev->product in wacom_setup_touch_input_capabilities Lee Jones
2026-09-02 13:44 ` [PATCH v9 2/5] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration Lee Jones
@ 2026-09-02 13:44 ` Lee Jones
2026-09-02 14:04 ` sashiko-bot
2026-09-02 13:44 ` [PATCH v9 4/5] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad Lee Jones
2026-09-02 13:44 ` [PATCH v9 5/5] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
3 siblings, 1 reply; 9+ messages in thread
From: Lee Jones @ 2026-09-02 13:44 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Dmitry Torokhov, Peter Hutterer, linux-input,
linux-kernel
Cc: stable
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 where a preempted reader in interrupt context
dereferences the freed pointer, leading to a Use-After-Free.
Resolve this by introducing RCU protection for the touch_input
pointer:
- Annotate 'touch_input' in wacom_shared struct with __rcu
- Wrap all lockless readers in wacom_wac.c with guard(rcu)() and
rcu_dereference() using a unified wacom_report_touch_mute()
helper
- Update writers in wacom_sys.c using rcu_assign_pointer()
- Call synchronize_rcu() in wacom_remove_shared_data() to ensure
all active RCU readers have finished before the input device is
freed
Also wrap wacom_set_shared_values() and touch/pen assignments in
wacom_add_shared_data() inside the wacom_udev_list_lock to serialize
concurrent probe assignments, and verify that 'shared->touch == hdev'
before setting touch_input to prevent concurrent sibling probe state
desynchronization.
Cc: stable@vger.kernel.org
Fixes: 961794a00eab ("Input: wacom - add reporting of SW_MUTE_DEVICE events")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
v3 -> v4: Dmitry's review [redundant check and guard()]
v4 -> v5: Jason's review [split, remove "awkward if"]
v5 -> v6: No change
v6 -> v7: No change
v7 -> v8: No functional change - cleaned up a stray whitespace/blank line deletion hunk
v8 -> v9: No change
drivers/hid/wacom_sys.c | 25 ++++++++++++++++++-------
drivers/hid/wacom_wac.c | 36 ++++++++++++++++++------------------
drivers/hid/wacom_wac.h | 2 +-
3 files changed, 37 insertions(+), 26 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 026a6be467d3..1af0d518260d 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;
+ }
+ }
+
+ synchronize_rcu();
kref_put(&data->kref, wacom_release_shared_data);
wacom_wac->shared = NULL;
@@ -912,6 +918,7 @@ static int wacom_add_shared_data(struct hid_device *hdev)
wacom_wac->shared = &data->shared;
retval = devm_add_action_or_reset(&hdev->dev, wacom_remove_shared_data, wacom);
+
return retval;
}
@@ -2337,6 +2344,8 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
{
struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+ guard(mutex)(&wacom_udev_list_lock);
+
if (wacom_wac->has_mute_touch_switch) {
wacom_wac->shared->has_mute_touch_switch = true;
/* Hardware touch switch may be off. Wait until
@@ -2349,9 +2358,11 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
}
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- wacom_wac->shared->type = wacom_wac->features.type;
- wacom_wac->shared->touch_input = wacom_wac->touch_input;
- wacom_wac->shared->touch = wacom->hdev;
+ if (wacom_wac->shared->touch == wacom->hdev || !wacom_wac->shared->touch) {
+ wacom_wac->shared->type = wacom_wac->features.type;
+ wacom_wac->shared->touch = wacom->hdev;
+ rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
+ }
} else if (wacom_wac->features.device_type &
(WACOM_DEVICETYPE_PEN | WACOM_DEVICETYPE_PAD)) {
wacom_wac->shared->pen = wacom->hdev;
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index a7e3817aa2c4..f953851ad6bc 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -510,6 +510,18 @@ static void wacom_intuos_schedule_prox_event(struct wacom_wac *wacom_wac)
}
}
+static void wacom_report_touch_mute(struct wacom_wac *wacom_wac, bool mute)
+{
+ struct input_dev *touch_input;
+
+ guard(rcu)();
+ touch_input = rcu_dereference(wacom_wac->shared->touch_input);
+ if (touch_input) {
+ input_report_switch(touch_input, SW_MUTE_DEVICE, mute);
+ input_sync(touch_input);
+ }
+}
+
static int wacom_intuos_pad(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
@@ -650,12 +662,8 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
input_report_key(input, KEY_CONTROLPANEL, menu);
input_report_key(input, KEY_INFO, info);
- if (wacom->shared && wacom->shared->touch_input) {
- input_report_switch(wacom->shared->touch_input,
- SW_MUTE_DEVICE,
- !wacom->shared->is_touch_on);
- input_sync(wacom->shared->touch_input);
- }
+ if (wacom->shared)
+ wacom_report_touch_mute(wacom, !wacom->shared->is_touch_on);
input_report_abs(input, ABS_RX, strip1);
input_report_abs(input, ABS_RY, strip2);
@@ -2155,7 +2163,7 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
*/
if ((equivalent_usage == WACOM_HID_WD_MUTE_DEVICE) ||
(equivalent_usage == WACOM_HID_WD_TOUCHONOFF)) {
- if (wacom_wac->shared->touch_input) {
+ if (wacom_wac->shared) {
bool *is_touch_on = &wacom_wac->shared->is_touch_on;
if (equivalent_usage == WACOM_HID_WD_MUTE_DEVICE && value)
@@ -2163,9 +2171,7 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
else if (equivalent_usage == WACOM_HID_WD_TOUCHONOFF)
*is_touch_on = value;
- input_report_switch(wacom_wac->shared->touch_input,
- SW_MUTE_DEVICE, !(*is_touch_on));
- input_sync(wacom_wac->shared->touch_input);
+ wacom_report_touch_mute(wacom_wac, !(*is_touch_on));
}
return;
}
@@ -3383,11 +3389,8 @@ static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
if ((wacom->shared->type == INTUOSHT ||
wacom->shared->type == INTUOSHT2) &&
- wacom->shared->touch_input &&
wacom->shared->touch_max) {
- input_report_switch(wacom->shared->touch_input,
- SW_MUTE_DEVICE, data[5] & 0x40);
- input_sync(wacom->shared->touch_input);
+ wacom_report_touch_mute(wacom, data[5] & 0x40);
}
pid = get_unaligned_be16(&data[6]);
@@ -3422,11 +3425,8 @@ static int wacom_status_irq(struct wacom_wac *wacom_wac, size_t len)
if ((features->type == INTUOSHT ||
features->type == INTUOSHT2) &&
- wacom_wac->shared->touch_input &&
features->touch_max) {
- input_report_switch(wacom_wac->shared->touch_input,
- SW_MUTE_DEVICE, data[8] & 0x40);
- input_sync(wacom_wac->shared->touch_input);
+ wacom_report_touch_mute(wacom_wac, data[8] & 0x40);
}
if (data[9] & 0x02) { /* wireless module is attached */
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 126bec6e5c0c..a8bbba4a6f37 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -285,7 +285,7 @@ struct wacom_shared {
/* for wireless device to access USB interfaces */
unsigned touch_max;
int type;
- struct input_dev *touch_input;
+ struct input_dev __rcu *touch_input;
struct hid_device *pen;
struct hid_device *touch;
bool has_mute_touch_switch;
--
2.55.0.966.g6673acef38-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v9 3/5] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
2026-09-02 13:44 ` [PATCH v9 3/5] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
@ 2026-09-02 14:04 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-02 14:04 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing cancellation of asynchronous work queues and timers in the probe error path after hardware start.
- [High] Lockless access to `shared->touch` and `shared->pen` in `wacom_mode_change_work` can lead to Use-After-Free during concurrent device removal.
--
commit 1b2b5cb79815a8638c58444b1176590871426b5f
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix Use-After-Free in wacom_intuos_pad
This patch addresses a Use-After-Free race condition in wacom_intuos_pad()
by adding RCU protection to the touch_input pointer. It also wraps device
assignments in wacom_udev_list_lock to serialize concurrent probe
assignments.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 026a6be467d3d..1af0d518260d6 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;
> + }
> + }
> +
> + synchronize_rcu();
>
> kref_put(&data->kref, wacom_release_shared_data);
> wacom_wac->shared = NULL;
[ ... ]
While reviewing this code, I noticed a couple of pre-existing issues in the
surrounding functions.
drivers/hid/wacom_sys.c:wacom_parse_and_register() {
...
error = hid_hw_start(hdev, connect_mask);
if (error) {
hid_err(hdev, "hw start failed\n");
goto fail;
}
error = wacom_register_inputs(wacom);
if (error)
goto fail_hw_stop;
...
fail_hw_stop:
hid_hw_stop(hdev);
fail:
wacom_release_resources(wacom);
return error;
}
[Severity: High]
This is a pre-existing issue and was not introduced by the patch under
review, but does this error path omit the cancellation of asynchronous
resources?
Once hid_hw_start() is called, the device hardware can generate interrupts
and schedule background work such as wireless_work, battery_work, or the
idleprox_timer. If a subsequent initialization step fails, the probe error
path calls hid_hw_stop() and wacom_release_resources(), and then returns.
This causes devres to free the wacom structure. Because the scheduled work
is not explicitly canceled using cancel_work_sync() or timer_delete_sync(),
the work could execute later and access the freed structure, leading to a
Use-After-Free.
drivers/hid/wacom_sys.c:wacom_mode_change_work() {
...
if (shared->pen) {
wacom1 = hid_get_drvdata(shared->pen);
wacom_release_resources(wacom1);
hid_hw_stop(wacom1->hdev);
wacom1->wacom_wac.has_mode_change = true;
...
}
[Severity: High]
This is a pre-existing issue and was not introduced by the patch under
review, but could this lockless access to sibling devices lead to a
Use-After-Free?
The wacom_mode_change_work function locklessly accesses sibling device
pointers like shared->pen and shared->touch. If the sibling device is
concurrently hot-unplugged and unbound, the synchronize_rcu() call in the
unbind path does not block this work from using a cached pointer, since
there is no RCU read-side critical section (such as guard(rcu)()). This
allows hid_get_drvdata() to dereference a freed device structure, resulting
in a Use-After-Free.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902134417.1270552-1-lee@kernel.org?part=3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v9 4/5] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
2026-09-02 13:44 [PATCH v9 1/5] HID: wacom: Use hdev->product in wacom_setup_touch_input_capabilities Lee Jones
2026-09-02 13:44 ` [PATCH v9 2/5] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration Lee Jones
2026-09-02 13:44 ` [PATCH v9 3/5] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
@ 2026-09-02 13:44 ` Lee Jones
2026-09-02 14:02 ` sashiko-bot
2026-09-02 13:44 ` [PATCH v9 5/5] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
3 siblings, 1 reply; 9+ messages in thread
From: Lee Jones @ 2026-09-02 13:44 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Dmitry Torokhov, Peter Hutterer, linux-input,
linux-kernel
Cc: stable
wacom_bamboo_pad_pen_event() accesses wacom->shared->pen locklessly
relative to wacom_remove_shared_data() which nullifies it. This
can lead to a Use-After-Free if the sibling device is removed while
events are being processed.
Resolve this by introducing RCU protection for pen and touch
pointers:
- Annotate 'pen' and 'touch' in wacom_shared struct with __rcu.
- Wrap lockless readers in wacom_bamboo_pad_pen_event() with
rcu_read_lock() and rcu_dereference().
- Update writers in wacom_sys.c using rcu_assign_pointer().
- Use rcu_dereference_protected for comparisons under
wacom_udev_list_lock.
- Also use rcu_access_pointer in wacom_mode_change_work() to avoid
warnings (while lockless access there remains a pre-existing issue).
Cc: stable@vger.kernel.org
Fixes: 8c97a765467c ("HID: wacom: add full support of the Wacom Bamboo PAD")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
v3 -> v4: Dmitry's review [guard()]
v4 -> v5: No change
v5 -> v6: No change
v6 -> v7: No change
v7 -> v8: No change
v8 -> v9: No change
drivers/hid/wacom_sys.c | 36 +++++++++++++++++++++++++-----------
drivers/hid/wacom_wac.c | 13 ++++++-------
drivers/hid/wacom_wac.h | 4 ++--
3 files changed, 33 insertions(+), 20 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 1af0d518260d..12f6a32cbc5f 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -876,11 +876,18 @@ static void wacom_remove_shared_data(void *res)
shared);
scoped_guard(mutex, &wacom_udev_list_lock) {
- if (wacom_wac->shared->touch == wacom->hdev) {
- wacom_wac->shared->touch = NULL;
+ struct hid_device *touch =
+ rcu_dereference_protected(wacom_wac->shared->touch,
+ lockdep_is_held(&wacom_udev_list_lock));
+ struct hid_device *pen =
+ rcu_dereference_protected(wacom_wac->shared->pen,
+ lockdep_is_held(&wacom_udev_list_lock));
+
+ if (touch == wacom->hdev) {
+ rcu_assign_pointer(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;
+ } else if (pen == wacom->hdev) {
+ rcu_assign_pointer(wacom_wac->shared->pen, NULL);
}
}
@@ -2358,14 +2365,18 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
}
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- if (wacom_wac->shared->touch == wacom->hdev || !wacom_wac->shared->touch) {
+ struct hid_device *touch =
+ rcu_dereference_protected(wacom_wac->shared->touch,
+ lockdep_is_held(&wacom_udev_list_lock));
+
+ if (touch == wacom->hdev || !touch) {
wacom_wac->shared->type = wacom_wac->features.type;
- wacom_wac->shared->touch = wacom->hdev;
+ rcu_assign_pointer(wacom_wac->shared->touch, wacom->hdev);
rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
}
} else if (wacom_wac->features.device_type &
(WACOM_DEVICETYPE_PEN | WACOM_DEVICETYPE_PAD)) {
- wacom_wac->shared->pen = wacom->hdev;
+ rcu_assign_pointer(wacom_wac->shared->pen, wacom->hdev);
}
}
@@ -2833,16 +2844,19 @@ static void wacom_mode_change_work(struct work_struct *work)
bool is_direct = wacom->wacom_wac.is_direct_mode;
int error = 0;
- if (shared->pen) {
- wacom1 = hid_get_drvdata(shared->pen);
+ struct hid_device *pen = rcu_access_pointer(shared->pen);
+ struct hid_device *touch = rcu_access_pointer(shared->touch);
+
+ 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;
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index f953851ad6bc..884d1cf7401c 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3294,6 +3294,7 @@ static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
unsigned char *data)
{
+ struct hid_device *pen;
unsigned char prefix;
/*
@@ -3306,13 +3307,11 @@ static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
prefix = data[0];
data[0] = WACOM_REPORT_BPAD_PEN;
- /*
- * actually reroute the event.
- * No need to check if wacom->shared->pen is valid, hid_input_report()
- * will check for us.
- */
- hid_input_report(wacom->shared->pen, HID_INPUT_REPORT, data,
- WACOM_PKGLEN_PENABLED, 1);
+ guard(rcu)();
+ pen = rcu_dereference(wacom->shared->pen);
+ if (pen)
+ hid_input_report(pen, HID_INPUT_REPORT, data,
+ WACOM_PKGLEN_PENABLED, 1);
data[0] = prefix;
}
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index a8bbba4a6f37..170d6adbe02a 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -286,8 +286,8 @@ struct wacom_shared {
unsigned touch_max;
int type;
struct input_dev __rcu *touch_input;
- struct hid_device *pen;
- struct hid_device *touch;
+ struct hid_device __rcu *pen;
+ struct hid_device __rcu *touch;
bool has_mute_touch_switch;
bool is_touch_on;
};
--
2.55.0.966.g6673acef38-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v9 4/5] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
2026-09-02 13:44 ` [PATCH v9 4/5] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad Lee Jones
@ 2026-09-02 14:02 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-02 14:02 UTC (permalink / raw)
To: Lee Jones; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Cross-device Use-After-Free in `wacom_mode_change_work()` due to lockless access to sibling devices without RCU read lock.
--
commit 4b7151c8603346fbd9de5581ed53af8776799547
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
This commit fixes a Use-After-Free in wacom_bamboo_pad_pen_event by introducing
RCU protection for pen and touch pointers. It adds rcu_read_lock() and
rcu_dereference() wrappers to lockless readers, while using rcu_access_pointer
in wacom_mode_change_work() to suppress warnings.
Cc: stable@vger.kernel.org
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 1af0d518260d6..12f6a32cbc5fc 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
[ ... ]
> @@ -2833,16 +2844,19 @@ static void wacom_mode_change_work(struct work_struct *work)
> bool is_direct = wacom->wacom_wac.is_direct_mode;
> int error = 0;
>
> - if (shared->pen) {
> - wacom1 = hid_get_drvdata(shared->pen);
> + struct hid_device *pen = rcu_access_pointer(shared->pen);
> + struct hid_device *touch = rcu_access_pointer(shared->touch);
> +
> + if (pen) {
> + wacom1 = hid_get_drvdata(pen);
[Severity: Critical]
This is a pre-existing issue, but can this lockless access to sibling devices
result in a cross-device Use-After-Free?
If wacom_mode_change_work() runs on a sibling device (e.g., touch) and reads
shared->pen using rcu_access_pointer() without holding rcu_read_lock():
1. Concurrently, the pen device is physically disconnected or unbound.
2. The pen device's unbind path calls wacom_remove_shared_data(), which
nullifies shared->pen and calls synchronize_rcu().
3. Because wacom_mode_change_work() does not hold rcu_read_lock(),
synchronize_rcu() does not block, allowing the pen device's unbind to
complete.
4. The pen device's struct wacom is freed via devres (devm_kzalloc in probe).
5. wacom_mode_change_work() then proceeds to dereference the freed pointer
via hid_get_drvdata(pen) and calls wacom_release_resources(wacom1) and
hid_hw_stop(wacom1->hdev) on it.
While the commit message notes this lockless access is a pre-existing issue,
could this still be triggered during a hot-unplug of a composite device during
a mode change event?
> wacom_release_resources(wacom1);
> hid_hw_stop(wacom1->hdev);
> wacom1->wacom_wac.has_mode_change = true;
> wacom1->wacom_wac.is_direct_mode = is_direct;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902134417.1270552-1-lee@kernel.org?part=4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v9 5/5] HID: wacom: Redesign shared sibling data lifecycle
2026-09-02 13:44 [PATCH v9 1/5] HID: wacom: Use hdev->product in wacom_setup_touch_input_capabilities Lee Jones
` (2 preceding siblings ...)
2026-09-02 13:44 ` [PATCH v9 4/5] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad Lee Jones
@ 2026-09-02 13:44 ` Lee Jones
2026-09-02 14:05 ` sashiko-bot
3 siblings, 1 reply; 9+ messages in thread
From: Lee Jones @ 2026-09-02 13:44 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Dmitry Torokhov, Peter Hutterer, linux-input,
linux-kernel
Cc: stable
The Wacom driver coordinates state between sibling interfaces of
the same physical device using a shared structure 'wacom_shared'
inside 'wacom_hdev_data'. The driver kept a volatile representative
pointer 'data->dev' pointing to a sibling 'hid_device' for physical
path comparisons during sibling matching.
This pointer management is fragile. When the representative device
is disconnected, wacom_remove_shared_data() failed to clear/update
'data->dev', leading to a Use-After-Free vulnerability when
subsequent sibling probes dereference the dangling 'data->dev'
pointer.
Resolve this issue by redesigning the sibling data lifecycle:
- Eliminate the volatile 'data->dev' representative pointer
completely
- Redesign 'wacom_hdev_data' to store stable static copies of the
required attributes upon first allocation: 'phys' path string,
'vendor', 'product' IDs and the sibling's 'device_type'
- Use these static attributes for stable sibling matching in
wacom_are_sibling() and wacom_get_hdev_data()
This ensures sibling matching remains safe and stable even if
individual siblings are dynamically added or removed.
To secure the lifecycle against concurrent probe/disconnect races:
- Switch kref_put() to kref_put_mutex() in
wacom_remove_shared_data() to serialize refcount drops with list
traversal and lookup
- Modify wacom_release_shared_data() to assume the list lock is
already held
Also, do not accumulate the 'device_type' capability flag during
subsequent sibling probes. Keeping only the first probed sibling's
device_type exactly preserves the original sibling matching behavior
without introducing side effects.
Cc: stable@vger.kernel.org
Fixes: 4492efffffeb ("Input: wacom - share pen info with touch of the same ID")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
v3 -> v4: No change
v4 -> v5: No change
v5 -> v6: No change
v6 -> v7: No change
v7 -> v8: No change
v8 -> v9: No change
drivers/hid/wacom_sys.c | 70 +++++++++++++++++++++++++++--------------
1 file changed, 46 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 12f6a32cbc5f..9b991078a36d 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -757,27 +757,47 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
struct wacom_hdev_data {
struct list_head list;
struct kref kref;
- struct hid_device *dev;
+ char phys[64];
+ __u32 vendor;
+ __u32 product;
+ __u32 device_type;
struct wacom_shared shared;
};
+static bool wacom_compare_device_paths(struct hid_device *hdev_a,
+ const char *phys_b, char separator)
+{
+ const char *p1 = strrchr(hdev_a->phys, separator);
+ const char *p2 = strrchr(phys_b, separator);
+ int n1, n2;
+
+ if (!p1 || !p2)
+ return false;
+
+ n1 = p1 - hdev_a->phys;
+ n2 = p2 - phys_b;
+
+ if (n1 != n2 || n1 <= 0 || n2 <= 0)
+ return false;
+
+ return !strncmp(hdev_a->phys, phys_b, n1);
+}
+
static LIST_HEAD(wacom_udev_list);
static DEFINE_MUTEX(wacom_udev_list_lock);
static bool wacom_are_sibling(struct hid_device *hdev,
- struct hid_device *sibling)
+ struct wacom_hdev_data *data)
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_features *features = &wacom->wacom_wac.features;
- struct wacom *sibling_wacom = hid_get_drvdata(sibling);
- struct wacom_features *sibling_features = &sibling_wacom->wacom_wac.features;
__u32 oVid = features->oVid ? features->oVid : hdev->vendor;
__u32 oPid = features->oPid ? features->oPid : hdev->product;
/* The defined oVid/oPid must match that of the sibling */
- if (features->oVid != HID_ANY_ID && sibling->vendor != oVid)
+ if (features->oVid != HID_ANY_ID && data->vendor != oVid)
return false;
- if (features->oPid != HID_ANY_ID && sibling->product != oPid)
+ if (features->oPid != HID_ANY_ID && data->product != oPid)
return false;
/*
@@ -785,11 +805,11 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* device path, while those with different VID/PID must share
* the same physical parent device path.
*/
- if (hdev->vendor == sibling->vendor && hdev->product == sibling->product) {
- if (!hid_compare_device_paths(hdev, sibling, '/'))
+ if (hdev->vendor == data->vendor && hdev->product == data->product) {
+ if (!wacom_compare_device_paths(hdev, data->phys, '/'))
return false;
} else {
- if (!hid_compare_device_paths(hdev, sibling, '.'))
+ if (!wacom_compare_device_paths(hdev, data->phys, '.'))
return false;
}
@@ -802,7 +822,7 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if ((features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ !(data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/*
@@ -810,17 +830,17 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if (!(features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- (sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ (data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/* Pen devices may only be siblings of touch devices */
if ((features->device_type & WACOM_DEVICETYPE_PEN) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_TOUCH))
+ !(data->device_type & WACOM_DEVICETYPE_TOUCH))
return false;
/* Touch devices may only be siblings of pen devices */
if ((features->device_type & WACOM_DEVICETYPE_TOUCH) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_PEN))
+ !(data->device_type & WACOM_DEVICETYPE_PEN))
return false;
/*
@@ -836,7 +856,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Try to find an already-probed interface from the same device */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (hid_compare_device_paths(hdev, data->dev, '/')) {
+ if (wacom_compare_device_paths(hdev, data->phys, '/')) {
kref_get(&data->kref);
return data;
}
@@ -844,7 +864,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Fallback to finding devices that appear to be "siblings" */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (wacom_are_sibling(hdev, data->dev)) {
+ if (wacom_are_sibling(hdev, data)) {
kref_get(&data->kref);
return data;
}
@@ -858,18 +878,15 @@ static void wacom_release_shared_data(struct kref *kref)
struct wacom_hdev_data *data =
container_of(kref, struct wacom_hdev_data, kref);
- mutex_lock(&wacom_udev_list_lock);
list_del(&data->list);
- mutex_unlock(&wacom_udev_list_lock);
-
kfree(data);
}
static void wacom_remove_shared_data(void *res)
{
- struct wacom *wacom = res;
+ struct wacom *res_wacom = res;
struct wacom_hdev_data *data;
- struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+ struct wacom_wac *wacom_wac = &res_wacom->wacom_wac;
if (wacom_wac->shared) {
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
@@ -883,17 +900,19 @@ static void wacom_remove_shared_data(void *res)
rcu_dereference_protected(wacom_wac->shared->pen,
lockdep_is_held(&wacom_udev_list_lock));
- if (touch == wacom->hdev) {
+ if (touch == res_wacom->hdev) {
rcu_assign_pointer(wacom_wac->shared->touch, NULL);
rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
- } else if (pen == wacom->hdev) {
+ } else if (pen == res_wacom->hdev) {
rcu_assign_pointer(wacom_wac->shared->pen, NULL);
}
}
synchronize_rcu();
- kref_put(&data->kref, wacom_release_shared_data);
+ if (kref_put_mutex(&data->kref, wacom_release_shared_data, &wacom_udev_list_lock))
+ mutex_unlock(&wacom_udev_list_lock);
+
wacom_wac->shared = NULL;
}
}
@@ -916,7 +935,10 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
kref_init(&data->kref);
- data->dev = hdev;
+ strscpy(data->phys, hdev->phys, sizeof(data->phys));
+ data->vendor = hdev->vendor;
+ data->product = hdev->product;
+ data->device_type = wacom_wac->features.device_type;
list_add_tail(&data->list, &wacom_udev_list);
}
--
2.55.0.966.g6673acef38-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v9 5/5] HID: wacom: Redesign shared sibling data lifecycle
2026-09-02 13:44 ` [PATCH v9 5/5] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
@ 2026-09-02 14:05 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-02 14:05 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free in wacom_mode_change_work due to unsafe RCU access and lack of sibling synchronization.
--
commit 144702a97dac139acf4377ec2bc37fa17f37b11a
Author: Lee Jones <lee@kernel.org>
HID: wacom: Redesign shared sibling data lifecycle
The Wacom driver coordinates state between sibling interfaces of
the same physical device using a shared structure 'wacom_shared'.
This redesigns the sibling data lifecycle by eliminating a volatile
representative pointer and storing stable static copies of the required
attributes for sibling matching. It also switches to kref_put_mutex()
to secure the lifecycle against concurrent probe/disconnect races.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 12f6a32cbc5fc..9b991078a36d8 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
[ ... ]
> @@ -883,17 +900,19 @@ static void wacom_remove_shared_data(void *res)
> rcu_dereference_protected(wacom_wac->shared->pen,
> lockdep_is_held(&wacom_udev_list_lock));
>
> - if (touch == wacom->hdev) {
> + if (touch == res_wacom->hdev) {
> rcu_assign_pointer(wacom_wac->shared->touch, NULL);
> rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
> - } else if (pen == wacom->hdev) {
> + } else if (pen == res_wacom->hdev) {
> rcu_assign_pointer(wacom_wac->shared->pen, NULL);
> }
> }
>
> synchronize_rcu();
[Severity: High]
This is a pre-existing issue, but does synchronize_rcu() properly protect
against concurrent access in wacom_mode_change_work()?
When a sibling device is removed, this devres cleanup clears the sibling
pointers and calls synchronize_rcu(). However, wacom_mode_change_work()
retrieves the sibling pointer outside of an RCU critical section:
struct hid_device *pen = rcu_access_pointer(shared->pen);
struct hid_device *touch = rcu_access_pointer(shared->touch);
if (pen) {
wacom1 = hid_get_drvdata(pen);
wacom_release_resources(wacom1);
hid_hw_stop(wacom1->hdev);
Since wacom_mode_change_work() executes in a sleepable workqueue context and
uses rcu_access_pointer() instead of rcu_dereference() inside an
rcu_read_lock(), does synchronize_rcu() actually wait for the workqueue to
finish?
If wacom_mode_change_work() is executing concurrently with sibling removal,
could it dereference a dangling sibling pointer leading to a use-after-free
when the sibling's devres cleanup frees the memory?
>
> - kref_put(&data->kref, wacom_release_shared_data);
> + if (kref_put_mutex(&data->kref, wacom_release_shared_data, &wacom_udev_list_lock))
> + mutex_unlock(&wacom_udev_list_lock);
> +
> wacom_wac->shared = NULL;
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902134417.1270552-1-lee@kernel.org?part=5
^ permalink raw reply [flat|nested] 9+ messages in thread