* [PATCH 2/5] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration
2026-08-25 15:12 [PATCH 1/5] HID: wacom: Use hdev->product in wacom_setup_touch_input_capabilities Lee Jones
@ 2026-08-25 15:12 ` Lee Jones
2026-08-25 15:12 ` [PATCH 3/5] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Lee Jones @ 2026-08-25 15:12 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Dmitry Torokhov, Peter Hutterer, linux-input,
linux-kernel
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.
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
drivers/hid/wacom_sys.c | 46 ++++++++++++++++++++++++++++++++++++-----
drivers/hid/wacom_wac.c | 4 ++++
2 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0eafa483b7f7..2738d4f515e6 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2358,13 +2358,44 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
if (wacom_wac->is_soft_touch_switch)
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);
+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)
@@ -2444,6 +2475,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.887.g758fc8c411-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/5] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
2026-08-25 15:12 [PATCH 1/5] HID: wacom: Use hdev->product in wacom_setup_touch_input_capabilities Lee Jones
2026-08-25 15:12 ` [PATCH 2/5] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration Lee Jones
@ 2026-08-25 15:12 ` Lee Jones
2026-08-25 15:12 ` [PATCH 4/5] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad Lee Jones
2026-08-25 15:12 ` [PATCH 5/5] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
3 siblings, 0 replies; 5+ messages in thread
From: Lee Jones @ 2026-08-25 15:12 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Peter Hutterer, Dmitry Torokhov, 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 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.
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
drivers/hid/wacom_sys.c | 34 +++++++++++++++++++++++-----------
drivers/hid/wacom_wac.c | 36 ++++++++++++++++++------------------
drivers/hid/wacom_wac.h | 2 +-
3 files changed, 42 insertions(+), 30 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2738d4f515e6..b88726cbbb4e 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;
@@ -907,6 +913,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;
@@ -915,11 +926,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;
}
@@ -2343,9 +2349,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);
+
+ guard(mutex)(&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) {
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.887.g758fc8c411-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 4/5] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
2026-08-25 15:12 [PATCH 1/5] HID: wacom: Use hdev->product in wacom_setup_touch_input_capabilities Lee Jones
2026-08-25 15:12 ` [PATCH 2/5] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration Lee Jones
2026-08-25 15:12 ` [PATCH 3/5] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
@ 2026-08-25 15:12 ` Lee Jones
2026-08-25 15:12 ` [PATCH 5/5] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
3 siblings, 0 replies; 5+ messages in thread
From: Lee Jones @ 2026-08-25 15:12 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, Dmitry Torokhov, Peter Hutterer, linux-input,
linux-kernel
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).
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
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 b88726cbbb4e..79fd397e044a 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);
}
}
@@ -914,9 +921,9 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
- data->shared.touch = hdev;
+ rcu_assign_pointer(data->shared.touch, hdev);
else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
- data->shared.pen = hdev;
+ rcu_assign_pointer(data->shared.pen, hdev);
mutex_unlock(&wacom_udev_list_lock);
@@ -2354,7 +2361,11 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
guard(mutex)(&wacom_udev_list_lock);
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- if (wacom_wac->shared->touch == wacom->hdev) {
+ struct hid_device *touch =
+ rcu_dereference_protected(wacom_wac->shared->touch,
+ lockdep_is_held(&wacom_udev_list_lock));
+
+ if (touch == wacom->hdev) {
wacom_wac->shared->type = wacom_wac->features.type;
rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
}
@@ -2836,16 +2847,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.887.g758fc8c411-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 5/5] HID: wacom: Redesign shared sibling data lifecycle
2026-08-25 15:12 [PATCH 1/5] HID: wacom: Use hdev->product in wacom_setup_touch_input_capabilities Lee Jones
` (2 preceding siblings ...)
2026-08-25 15:12 ` [PATCH 4/5] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad Lee Jones
@ 2026-08-25 15:12 ` Lee Jones
3 siblings, 0 replies; 5+ messages in thread
From: Lee Jones @ 2026-08-25 15:12 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Aaron Skomra, 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
v2 -> v3: Sashiko fixes
v3 -> v4: No change
v4 -> v5: No change
v5 -> v6: No change
v6 -> v7: No change
v7 -> v8: 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 79fd397e044a..d4b6dc9eb0c8 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.887.g758fc8c411-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread