* [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-08 22:20 ` Ping Cheng
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, 2 replies; 11+ 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] 11+ 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
2026-09-08 22:20 ` Ping Cheng
1 sibling, 0 replies; 11+ 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] 11+ 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
@ 2026-09-08 22:20 ` Ping Cheng
2026-09-09 15:59 ` Lee Jones
1 sibling, 1 reply; 11+ messages in thread
From: Ping Cheng @ 2026-09-08 22:20 UTC (permalink / raw)
To: Lee Jones
Cc: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, Dmitry Torokhov, linux-input, linux-kernel,
stable
I tested the devices that I can reach, mainly the ones released in the
past 10 years or so. The tablets all worked. No crashes whatsoever. I
am comfortable to give the whole set of this version an acked-by and a
tested-by to help it move forward.
Acked-by: Ping Cheng <ping.cheng@wacom.com>
Tested-by: Ping Cheng <ping.cheng@wacom.com>
With that said, the SW_MUTE_DEVICE capability assigned through a
softkey is not properly recognized. I've figured out the root cause. I
will submit a patch after this set is merged. The main value of this
set is to resolve the Use-After-Free issue and the redesign of the
shared sibling data lifecycle. The individual softkey feature can be
addressed separately.
Hope my testing results make the maintainers feel comfortable too.
Thank you,
Ping
On Wed, Sep 2, 2026 at 8:20 AM Lee Jones <lee@kernel.org> wrote:
>
> 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 [flat|nested] 11+ messages in thread* Re: [PATCH v9 2/5] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration
2026-09-08 22:20 ` Ping Cheng
@ 2026-09-09 15:59 ` Lee Jones
0 siblings, 0 replies; 11+ messages in thread
From: Lee Jones @ 2026-09-09 15:59 UTC (permalink / raw)
To: Ping Cheng
Cc: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, Dmitry Torokhov, linux-input, linux-kernel,
stable
On Tue, 08 Sep 2026, Ping Cheng wrote:
> I tested the devices that I can reach, mainly the ones released in the
> past 10 years or so. The tablets all worked. No crashes whatsoever. I
> am comfortable to give the whole set of this version an acked-by and a
> tested-by to help it move forward.
>
> Acked-by: Ping Cheng <ping.cheng@wacom.com>
> Tested-by: Ping Cheng <ping.cheng@wacom.com>
>
> With that said, the SW_MUTE_DEVICE capability assigned through a
> softkey is not properly recognized. I've figured out the root cause. I
> will submit a patch after this set is merged. The main value of this
> set is to resolve the Use-After-Free issue and the redesign of the
> shared sibling data lifecycle. The individual softkey feature can be
> addressed separately.
>
> Hope my testing results make the maintainers feel comfortable too.
Thanks for these.
Sorry, I didn't see them before submitting v10 this morning.
--
Lee Jones
^ permalink raw reply [flat|nested] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ messages in thread