* [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
@ 2026-06-01 14:08 Lee Jones
2026-06-01 14:08 ` [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Lee Jones @ 2026-06-01 14:08 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Dmitry Torokhov, Peter Hutterer, linux-input, linux-kernel
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 lockless readers in wacom_wac.c with rcu_read_lock() and rcu_dereference()
- 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
To ensure the devres release order is correct and wacom_remove_shared_data()
completes its RCU synchronization BEFORE the input device is freed, move the
wacom_add_shared_data() call to the very end of wacom_parse_and_register().
Since devres actions are executed in LIFO order, this guarantees the shared
reference is cleared and synchronized before the input device resource is destroyed.
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.
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
drivers/hid/wacom_sys.c | 49 +++++++++++++++++++++++++++--------------
drivers/hid/wacom_wac.c | 17 +++++++++-----
drivers/hid/wacom_wac.h | 2 +-
3 files changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2220168bf116..26fce6d3198a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -877,10 +877,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)
+ mutex_lock(&wacom_udev_list_lock);
+ if (wacom_wac->shared->touch == wacom->hdev) {
wacom_wac->shared->touch = NULL;
- else if (wacom_wac->shared->pen == wacom->hdev)
+ rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
+ } else if (wacom_wac->shared->pen == wacom->hdev) {
wacom_wac->shared->pen = NULL;
+ }
+ mutex_unlock(&wacom_udev_list_lock);
+
+ synchronize_rcu();
kref_put(&data->kref, wacom_release_shared_data);
wacom_wac->shared = NULL;
@@ -909,6 +915,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)
list_add_tail(&data->list, &wacom_udev_list);
}
+ if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
+ data->shared.touch = hdev;
+ else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
+ data->shared.pen = hdev;
+
mutex_unlock(&wacom_udev_list_lock);
wacom_wac->shared = &data->shared;
@@ -917,11 +928,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
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;
}
@@ -2345,9 +2351,15 @@ static void wacom_release_resources(struct wacom *wacom)
static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
{
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+
+ mutex_lock(&wacom_udev_list_lock);
+
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;
+ if (wacom_wac->shared->touch == wacom->hdev) {
+ wacom_wac->shared->type = wacom_wac->features.type;
+ rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
+ }
}
if (wacom_wac->has_mute_touch_switch) {
@@ -2362,11 +2374,14 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
}
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);
+ rcu_access_pointer(wacom_wac->shared->touch_input)) {
+ struct input_dev *touch_input = rcu_dereference_protected(wacom_wac->shared->touch_input,
+ lockdep_is_held(&wacom_udev_list_lock));
+ set_bit(EV_SW, touch_input->evbit);
+ input_set_capability(touch_input, EV_SW, SW_MUTE_DEVICE);
}
+
+ mutex_unlock(&wacom_udev_list_lock);
}
static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
@@ -2442,10 +2457,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail;
}
- error = wacom_add_shared_data(hdev);
- if (error)
- goto fail;
-
error = wacom_setup_inputs(wacom);
if (error)
goto fail;
@@ -2496,6 +2507,10 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
}
}
+ error = wacom_add_shared_data(hdev);
+ if (error)
+ goto fail_quirks;
+
wacom_set_shared_values(wacom_wac);
devres_close_group(&hdev->dev, wacom);
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..c7cf1aaab903 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -650,11 +650,18 @@ 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) {
+ struct input_dev *touch_input;
+
+ rcu_read_lock();
+ touch_input = rcu_dereference(wacom->shared->touch_input);
+ if (touch_input) {
+ input_report_switch(touch_input,
+ SW_MUTE_DEVICE,
+ !wacom->shared->is_touch_on);
+ input_sync(touch_input);
+ }
+ rcu_read_unlock();
}
input_report_abs(input, ABS_RX, strip1);
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.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle
2026-06-01 14:08 [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
@ 2026-06-01 14:08 ` Lee Jones
2026-06-01 14:26 ` [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad sashiko-bot
2026-06-01 14:48 ` Markus Elfring
2 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2026-06-01 14:08 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Dmitry Torokhov, Peter Hutterer, linux-input, linux-kernel
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.
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
drivers/hid/wacom_sys.c | 63 +++++++++++++++++++++++++----------------
1 file changed, 39 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 26fce6d3198a..7cd0b0ff3128 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -759,27 +759,40 @@ 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)
+{
+ int n1 = strrchr(hdev_a->phys, separator) - hdev_a->phys;
+ int n2 = strrchr(phys_b, separator) - 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;
/*
@@ -787,11 +800,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;
}
@@ -804,7 +817,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;
/*
@@ -812,17 +825,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;
/*
@@ -838,7 +851,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;
}
@@ -846,7 +859,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;
}
@@ -860,35 +873,34 @@ 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,
shared);
mutex_lock(&wacom_udev_list_lock);
- if (wacom_wac->shared->touch == wacom->hdev) {
+ if (wacom_wac->shared->touch == res_wacom->hdev) {
wacom_wac->shared->touch = NULL;
rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
- } else if (wacom_wac->shared->pen == wacom->hdev) {
+ } else if (wacom_wac->shared->pen == res_wacom->hdev) {
wacom_wac->shared->pen = NULL;
}
mutex_unlock(&wacom_udev_list_lock);
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;
}
}
@@ -911,7 +923,10 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
kref_init(&data->kref);
- data->dev = hdev;
+ strncpy(data->phys, hdev->phys, sizeof(data->phys) - 1);
+ 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.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
2026-06-01 14:08 [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
2026-06-01 14:08 ` [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
@ 2026-06-01 14:26 ` sashiko-bot
2026-06-01 14:48 ` Markus Elfring
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-01 14:26 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:
- [Critical] Moving wacom_add_shared_data() to the end of wacom_parse_and_register() causes an unconditional NULL pointer dereference during device probe.
- [High] Moving wacom_add_shared_data() to the end of the probe function creates a window where wacom_wac->shared is NULL while hardware interrupts are active, leading to an asynchronous NULL pointer dereference.
Pre-existing issues:
- [High] The patch incompletely fixes a Time-of-Check to Time-of-Use (TOCTOU) race condition. It successfully converts wacom_intuos_pad() to use RCU, but leaves identical lockless accesses unprotected in wacom_wac_pad_event(), wacom_wireless_irq(), and wacom_status_irq().
- [High] Modifying input capabilities on the sibling touch device after it has already been registered with the input core.
--
commit 5bec6e8c12bc84f46d120679cc4bef944e215dc2
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix Use-After-Free in wacom_intuos_pad
This commit resolves a Time-of-Check to Time-of-Use race condition by
introducing RCU protection for the touch_input pointer. It also delays the
call to wacom_add_shared_data() to ensure correct devres release order.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
[ ... ]
> @@ -2362,11 +2374,14 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
> }
>
> 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);
> + rcu_access_pointer(wacom_wac->shared->touch_input)) {
> + struct input_dev *touch_input = rcu_dereference_protected(wacom_wac->shared->touch_input,
> + lockdep_is_held(&wacom_udev_list_lock));
> + set_bit(EV_SW, touch_input->evbit);
> + input_set_capability(touch_input, EV_SW, SW_MUTE_DEVICE);
> }
> +
> + mutex_unlock(&wacom_udev_list_lock);
> }
[Severity: High]
This is a pre-existing issue, but does this capability modification happen
too late?
In wacom_set_shared_values(), input capabilities are modified on the sibling
touch device. Because the Pen interface probes after the Touch interface,
this occurs on a sibling's touch_input device that was already fully probed
and registered.
The input subsystem requires that all capabilities must be fully initialized
before calling input_register_device(). Modifying capabilities
post-registration means the ADD uevent has already been emitted, and
userspace components miss the capability changes and might fail to process
events like SW_MUTE_DEVICE.
[ ... ]
> @@ -2442,10 +2457,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> goto fail;
> }
>
> - error = wacom_add_shared_data(hdev);
> - if (error)
> - goto fail;
> -
> error = wacom_setup_inputs(wacom);
> if (error)
> goto fail;
[Severity: Critical]
Does this change introduce a regression during device probe?
By removing wacom_add_shared_data() from this earlier location,
wacom_wac->shared is left uninitialized (NULL). However, wacom_setup_inputs()
is called immediately after:
wacom_setup_inputs()
wacom_setup_touch_input_capabilities()
if (wacom_wac->shared->touch->product == 0x361) {
Because wacom_wac->shared is unconditionally dereferenced for some devices
like INTUOSP2_BT and WACOM_27QHDT inside wacom_setup_touch_input_capabilities(),
could this cause a NULL pointer dereference during initialization?
[ ... ]
> @@ -2496,6 +2507,10 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> }
> }
>
> + error = wacom_add_shared_data(hdev);
> + if (error)
> + goto fail_quirks;
> +
> wacom_set_shared_values(wacom_wac);
> devres_close_group(&hdev->dev, wacom);
[Severity: High]
Is there a race condition here since wacom_add_shared_data() is now called
at the very end of the probe function?
Before reaching this point, wacom_parse_and_register() has already called
hid_hw_start() and wacom_register_inputs(). hid_hw_start() enables incoming
HID reports and wacom_register_inputs() exposes the device to userspace.
If a hardware interrupt fires right after hid_hw_start(), the interrupt
handlers can execute while wacom_wac->shared is still NULL:
wacom_bpt_pen()
wacom->shared->stylus_in_proximity = true;
Could this cause an asynchronous NULL pointer dereference if a report
arrives before the shared data is allocated?
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
[ ... ]
> @@ -650,11 +650,18 @@ 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) {
> + struct input_dev *touch_input;
> +
> + rcu_read_lock();
> + touch_input = rcu_dereference(wacom->shared->touch_input);
> + if (touch_input) {
> + input_report_switch(touch_input,
> + SW_MUTE_DEVICE,
> + !wacom->shared->is_touch_on);
> + input_sync(touch_input);
> + }
> + rcu_read_unlock();
> }
[Severity: High]
This is a pre-existing issue, but did this patch miss a few other places
where touch_input is accessed locklessly?
This update successfully converts wacom_intuos_pad() to use RCU, but there
are other HID event handlers in wacom_wac.c that still directly dereference
wacom_wac->shared->touch_input without holding rcu_read_lock().
For example, in wacom_wac_pad_event():
if (wacom_wac->shared->touch_input) {
...
input_report_switch(wacom_wac->shared->touch_input, ...);
Similar direct accesses happen in wacom_wireless_irq() and
wacom_status_irq(). Because these readers do not establish an RCU read-side
critical section, synchronize_rcu() in wacom_remove_shared_data() will not
wait for them.
If the sibling touch device is unbound, could the input_dev be freed while
these handlers are still running, resulting in the same use-after-free issue?
Direct access to an __rcu pointer may also trigger Sparse warnings.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260601140845.1237227-1-lee@kernel.org?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
2026-06-01 14:08 [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
2026-06-01 14:08 ` [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
2026-06-01 14:26 ` [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad sashiko-bot
@ 2026-06-01 14:48 ` Markus Elfring
2 siblings, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2026-06-01 14:48 UTC (permalink / raw)
To: Lee Jones, linux-input, Benjamin Tissoires, Dmitry Torokhov,
Jason Gerecke, Jiri Kosina, Peter Hutterer, Ping Cheng
Cc: LKML
…
> +++ b/drivers/hid/wacom_sys.c
> @@ -877,10 +877,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)
> + mutex_lock(&wacom_udev_list_lock);
> + if (wacom_wac->shared->touch == wacom->hdev) {
> wacom_wac->shared->touch = NULL;
…
> + }
> + mutex_unlock(&wacom_udev_list_lock);
> +
> + synchronize_rcu();
>
> kref_put(&data->kref, wacom_release_shared_data);
…
Under which circumstances would you dare to apply another lock guard?
https://elixir.bootlin.com/linux/v7.1-rc5/source/include/linux/mutex.h#L253
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-01 14:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-01 14:08 [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
2026-06-01 14:08 ` [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
2026-06-01 14:26 ` [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad sashiko-bot
2026-06-01 14:48 ` Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox