[PATCH v4 1/3] HID: wacom: Fix Use-After-Free in wacom_intuos_pad

[PATCH v4 1/3] HID: wacom: Fix Use-After-Free in wacom_intuos_pad

[Lee Jones]

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 rcu_read_lock() 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.

Finally, advertise the SW_MUTE_DEVICE capability on Touch input
devices prior to registration in wacom_setup_touch_input_capabilities()
to prevent invalid post-registration capability modifications.

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()]

 drivers/hid/wacom_sys.c | 41 ++++++++++++++-----------
 drivers/hid/wacom_wac.c | 66 +++++++++++++++++++++--------------------
 drivers/hid/wacom_wac.h |  2 +-
 3 files changed, 58 insertions(+), 51 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2220168bf116..86895f13dbae 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)
-			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;
@@ -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);
+
+	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) {
@@ -2360,13 +2372,6 @@ 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 int wacom_parse_and_register(struct wacom *wacom, bool wireless)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..23eaa81cd827 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);
@@ -2153,7 +2161,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)
@@ -2161,9 +2169,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;
 	}
@@ -3381,11 +3387,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]);
@@ -3420,11 +3423,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 */
@@ -3951,11 +3951,22 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
 int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
 					 struct wacom_wac *wacom_wac)
 {
+	struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+	struct hid_device *hdev = wacom->hdev;
 	struct wacom_features *features = &wacom_wac->features;
 
 	if (!(features->device_type & WACOM_DEVICETYPE_TOUCH))
 		return -ENODEV;
 
+	if (features->type != TABLETPC &&
+	    features->type != TABLETPC2FG &&
+	    features->type != MTSCREEN &&
+	    features->type != MTTPC &&
+	    features->type != MTTPC_B) {
+		input_dev->evbit[0] |= BIT_MASK(EV_SW);
+		__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
+	}
+
 	if (features->device_type & WACOM_DEVICETYPE_DIRECT)
 		__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
 	else
@@ -3992,22 +4003,17 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
 	switch (features->type) {
 	case INTUOSP2_BT:
 	case INTUOSP2S_BT:
-		input_dev->evbit[0] |= BIT_MASK(EV_SW);
-		__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
-
-		if (wacom_wac->shared->touch->product == 0x361) {
+		if (hdev->product == 0x361) {
 			input_set_abs_params(input_dev, ABS_MT_POSITION_X,
 					     0, 12440, 4, 0);
 			input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
 					     0, 8640, 4, 0);
-		}
-		else if (wacom_wac->shared->touch->product == 0x360) {
+		} else if (hdev->product == 0x360) {
 			input_set_abs_params(input_dev, ABS_MT_POSITION_X,
 					     0, 8960, 4, 0);
 			input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
 					     0, 5920, 4, 0);
-		}
-		else if (wacom_wac->shared->touch->product == 0x393) {
+		} else if (hdev->product == 0x393) {
 			input_set_abs_params(input_dev, ABS_MT_POSITION_X,
 					     0, 6400, 4, 0);
 			input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
@@ -4037,10 +4043,8 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
 		fallthrough;
 
 	case WACOM_27QHDT:
-		if (wacom_wac->shared->touch->product == 0x32C ||
-		    wacom_wac->shared->touch->product == 0xF6) {
-			input_dev->evbit[0] |= BIT_MASK(EV_SW);
-			__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
+		if (hdev->product == 0x32C ||
+		    hdev->product == 0xF6) {
 			wacom_wac->has_mute_touch_switch = true;
 			wacom_wac->is_soft_touch_switch = true;
 		}
@@ -4059,8 +4063,6 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
 
 	case INTUOSHT:
 	case INTUOSHT2:
-		input_dev->evbit[0] |= BIT_MASK(EV_SW);
-		__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
 		fallthrough;
 
 	case BAMBOO_PT:
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.1136.gdb2ca164c4-goog

[PATCH v4 2/3] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad

[Lee Jones]

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>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
v3 -> v4: Dmitry's review [guard()]

 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 86895f13dbae..3990d8d0b40c 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -878,11 +878,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);
 			}
 		}
 
@@ -916,9 +923,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);
 
@@ -2356,7 +2363,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);
 		}
@@ -2795,16 +2806,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 23eaa81cd827..92d2f2f3f552 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3292,6 +3292,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;
 
 	/*
@@ -3304,13 +3305,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.54.0.1136.gdb2ca164c4-goog

[PATCH v4 3/3] HID: wacom: Redesign shared sibling data lifecycle

[Lee Jones]

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

 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 3990d8d0b40c..9fb9dde7dd73 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -759,27 +759,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;
 
 	/*
@@ -787,11 +807,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 +824,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 +832,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 +858,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 +866,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,18 +880,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,
@@ -885,17 +902,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;
 	}
 }
@@ -918,7 +937,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.54.0.1136.gdb2ca164c4-goog

Re: [PATCH v4 1/3] HID: wacom: Fix Use-After-Free in wacom_intuos_pad

[Jiri Kosina]

Lee,

thanks, it looks good to me now.

I'd still like to get an Ack from either Jason or Ping on this before 
applying, especially due to nature of patch #3.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v4 1/3] HID: wacom: Fix Use-After-Free in wacom_intuos_pad

[Jason Gerecke]

I'm still working on testing and reviewing this set as a whole, but I
can provide some immediate feedback on this first patch at least:

On Tue, Jun 16, 2026 at 2:29 AM Lee Jones <lee@kernel.org> wrote:
>
> 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 rcu_read_lock() 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.
>
> Finally, advertise the SW_MUTE_DEVICE capability on Touch input
> devices prior to registration in wacom_setup_touch_input_capabilities()
> to prevent invalid post-registration capability modifications.
>

It seems like this patch can be split into at least two different
patches, maybe even three. The changes to
wacom_setup_touch_input_capabilities() especially feel like they could
be seperate from the introduction of RCU for shared->touch_input.

> 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()]
>
>  drivers/hid/wacom_sys.c | 41 ++++++++++++++-----------
>  drivers/hid/wacom_wac.c | 66 +++++++++++++++++++++--------------------
>  drivers/hid/wacom_wac.h |  2 +-
>  3 files changed, 58 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 2220168bf116..86895f13dbae 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)
> -                       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;
> @@ -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);
> +
> +       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) {
> @@ -2360,13 +2372,6 @@ 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 int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625..23eaa81cd827 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);
> @@ -2153,7 +2161,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)
> @@ -2161,9 +2169,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;
>         }
> @@ -3381,11 +3387,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]);
> @@ -3420,11 +3423,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 */
> @@ -3951,11 +3951,22 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
>  int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
>                                          struct wacom_wac *wacom_wac)
>  {
> +       struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
> +       struct hid_device *hdev = wacom->hdev;
>         struct wacom_features *features = &wacom_wac->features;
>
>         if (!(features->device_type & WACOM_DEVICETYPE_TOUCH))
>                 return -ENODEV;
>
> +       if (features->type != TABLETPC &&
> +           features->type != TABLETPC2FG &&
> +           features->type != MTSCREEN &&
> +           features->type != MTTPC &&
> +           features->type != MTTPC_B) {
> +               input_dev->evbit[0] |= BIT_MASK(EV_SW);
> +               __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
> +       }
> +

1. This if statement is missing a number of cases. INTUOS5, for
example (and among others) is missing and would incorrectly gain
SW_MUTE_DEVICE. The list of cases from the big switch below needs to
be consulted again.

2. It seems like this would result in SW_MUTE_DEVICE being given to
all HID_GENERIC touch devices. It should only be added in the case
that `wacom_wac->has_mute_touch_switch` is true, however.

3. I'm not sure why we're even bothering with moving all of the
initialization for SW_MUTE_DEVICE from the individual cases out to a
single awkward if statement. The only thing I can think of is that
HID_GENERIC devices need to have it set (when appropriate) now that
the responsibility has been removed from wacom_set_shared_values(). It
should be much cleaner, however, to simply add the SW_MUTE_DEVICE
setup for HID_GENERIC devices right before the function returns early
for them.

>         if (features->device_type & WACOM_DEVICETYPE_DIRECT)
>                 __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
>         else
> @@ -3992,22 +4003,17 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
>         switch (features->type) {
>         case INTUOSP2_BT:
>         case INTUOSP2S_BT:
> -               input_dev->evbit[0] |= BIT_MASK(EV_SW);
> -               __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
> -
> -               if (wacom_wac->shared->touch->product == 0x361) {
> +               if (hdev->product == 0x361) {
>                         input_set_abs_params(input_dev, ABS_MT_POSITION_X,
>                                              0, 12440, 4, 0);
>                         input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
>                                              0, 8640, 4, 0);
> -               }
> -               else if (wacom_wac->shared->touch->product == 0x360) {
> +               } else if (hdev->product == 0x360) {
>                         input_set_abs_params(input_dev, ABS_MT_POSITION_X,
>                                              0, 8960, 4, 0);
>                         input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
>                                              0, 5920, 4, 0);
> -               }
> -               else if (wacom_wac->shared->touch->product == 0x393) {
> +               } else if (hdev->product == 0x393) {
>                         input_set_abs_params(input_dev, ABS_MT_POSITION_X,
>                                              0, 6400, 4, 0);
>                         input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> @@ -4037,10 +4043,8 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
>                 fallthrough;
>
>         case WACOM_27QHDT:
> -               if (wacom_wac->shared->touch->product == 0x32C ||
> -                   wacom_wac->shared->touch->product == 0xF6) {
> -                       input_dev->evbit[0] |= BIT_MASK(EV_SW);
> -                       __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
> +               if (hdev->product == 0x32C ||
> +                   hdev->product == 0xF6) {
>                         wacom_wac->has_mute_touch_switch = true;
>                         wacom_wac->is_soft_touch_switch = true;
>                 }
> @@ -4059,8 +4063,6 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
>
>         case INTUOSHT:
>         case INTUOSHT2:
> -               input_dev->evbit[0] |= BIT_MASK(EV_SW);
> -               __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
>                 fallthrough;
>
>         case BAMBOO_PT:
> 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.1136.gdb2ca164c4-goog
>
>

Re: [PATCH v4 1/3] HID: wacom: Fix Use-After-Free in wacom_intuos_pad

[Lee Jones]

On Tue, 30 Jun 2026, Jason Gerecke wrote:

> I'm still working on testing and reviewing this set as a whole, but I
> can provide some immediate feedback on this first patch at least:

v5 posted.  I look forward to your review.

-- 
Lee Jones