[PATCH v7 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration

[PATCH v7 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration

[Lee Jones]

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, including composite USB generic touch devices.

Additionally, replace the lookup-dependent
'wacom_wac->shared->touch->product' references with 'hdev->product'
inside wacom_setup_touch_input_capabilities() as 'hdev' is already
available and represents the touch device itself.

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

 drivers/hid/wacom_sys.c | 23 +++++++++++++++++------
 drivers/hid/wacom_wac.c | 19 +++++++++++--------
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0eafa483b7f7..92b73ed0028f 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2359,12 +2359,6 @@ 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);
-	}
 }
 
 static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
@@ -2414,6 +2408,23 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 	wacom_retrieve_hid_descriptor(hdev, features);
 	wacom_setup_device_quirks(wacom);
 
+	if (features->type == HID_GENERIC &&
+	    (features->device_type & WACOM_DEVICETYPE_TOUCH)) {
+		if (wacom->usbdev && wacom->usbdev->actconfig &&
+		    wacom->usbdev->actconfig->desc.bNumInterfaces > 1) {
+			/*
+			 * Heuristic: Composite USB devices (like tablets with
+			 * pen/pad + touch) likely have a touch mute switch.
+			 * We flag it here to advertise the capability before
+			 * registration. We also set is_soft_touch_switch to
+			 * default touch to ON in case there is no physical
+			 * switch.
+			 */
+			wacom_wac->has_mute_touch_switch = true;
+			wacom_wac->is_soft_touch_switch = true;
+		}
+	}
+
 	if (features->device_type == WACOM_DEVICETYPE_NONE &&
 	    features->type != WIRELESS) {
 		error = features->type == HID_GENERIC ? -ENODEV : 0;
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index a29bf051ada7..afc82fcbb20b 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3953,6 +3953,8 @@ 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))
@@ -3963,9 +3965,12 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
 	else
 		__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
 
-	if (features->type == HID_GENERIC)
+	if (features->type == HID_GENERIC) {
+		if (wacom_wac->has_mute_touch_switch)
+			input_set_capability(input_dev, EV_SW, SW_MUTE_DEVICE);
 		/* setup has already been done */
 		return 0;
+	}
 
 	input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 	__set_bit(BTN_TOUCH, input_dev->keybit);
@@ -3997,19 +4002,17 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
 		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,
@@ -4039,8 +4042,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) {
+		if (hdev->product == 0x32C ||
+		    hdev->product == 0xF6) {
 			input_dev->evbit[0] |= BIT_MASK(EV_SW);
 			__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
 			wacom_wac->has_mute_touch_switch = true;
-- 
2.55.0.571.g244d577d93-goog

[PATCH v7 2/4] 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 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

 drivers/hid/wacom_sys.c | 35 +++++++++++++++++++++++------------
 drivers/hid/wacom_wac.c | 36 ++++++++++++++++++------------------
 drivers/hid/wacom_wac.h |  2 +-
 3 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 92b73ed0028f..cf9c12f3cff5 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) {
@@ -2358,7 +2370,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;
 	}
-
 }
 
 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 afc82fcbb20b..6ae04630eddc 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.571.g244d577d93-goog

[PATCH v7 3/4] 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>
---

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

 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 cf9c12f3cff5..05d27fcd311f 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);
 		}
@@ -2810,16 +2821,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 6ae04630eddc..ba9d6c1291fc 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.571.g244d577d93-goog

[PATCH v7 4/4] 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
v4 -> v5: No change
v5 -> v6: No change
v6 -> v7: 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 05d27fcd311f..b7008aae9bce 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.571.g244d577d93-goog

Re: [PATCH v7 2/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless access to sibling `hid_device` pointers (`shared->touch` and `shared->pen`) in `wacom_mode_change_work()` causes a TOCTOU race leading to Use-After-Free if a sibling device is disconnected concurrently.
- [High] Race condition between `kref_put()` in `wacom_remove_shared_data()` and `kref_get()` in `wacom_get_hdev_data()` leads to Use-After-Free of the shared data structure.
--

--- Patch [2]: [PATCH v7 2/4] HID: wacom: Fix Use-After-Free in wacom_intuos_pad ---
commit 17f29646966a71681ee3c27f7476ff3314a2db05
Author: Lee Jones <lee@kernel.org>

HID: wacom: Fix Use-After-Free in wacom_intuos_pad

This commit introduces RCU protection for the touch_input pointer to resolve
a TOCTOU race condition in wacom_intuos_pad(). It also serializes probe
assignments and removal in the shared data struct using wacom_udev_list_lock.

> 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
> @@ -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;
> +			}

[Severity: High]
This isn't a bug introduced by this patch, but it looks like shared->pen and
shared->touch can still be accessed locklessly in wacom_mode_change_work().

If the sibling pen device is dynamically unbound via sysfs or unplugged,
this path clears the pointer under the lock, frees the wacom structure,
and drops the hid_device reference.

Could a preempted wacom_mode_change_work() worker thread read the valid
shared->pen pointer before it is cleared, resume after the unbind completes,
and then dereference the freed pointer?

    if (shared->pen) {
        wacom1 = hid_get_drvdata(shared->pen);
        wacom_release_resources(wacom1);
        hid_hw_stop(wacom1->hdev);
    }

> +		}
> +
> +		synchronize_rcu();
>  
>  		kref_put(&data->kref, wacom_release_shared_data);

[Severity: High]
This is a pre-existing issue, but I have a question about this kref_put()
call happening outside of the wacom_udev_list_lock.

If the refcount drops to 0, wacom_release_shared_data() is queued and will
block on acquiring wacom_udev_list_lock to remove the data object from
wacom_udev_list.

Concurrently, if wacom_add_shared_data() is probing on another CPU, it holds
the mutex and calls wacom_get_hdev_data(). That function iterates the list
and calls kref_get() unconditionally on the matching item:

    list_for_each_entry(data, &wacom_udev_list, list) {
        if (hid_compare_device_paths(hdev, data->dev, '/')) {
            kref_get(&data->kref);
            return data;
        }
    }

Does this create a race where wacom_get_hdev_data() can acquire a reference
to a 0-refcount object just before wacom_release_shared_data() removes it
and calls kfree(data), leading to a use-after-free in the concurrent probe?
Should this perhaps use kref_put_mutex() to ensure atomicity?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804103209.1496683-1-lee@kernel.org?part=2

Re: [PATCH v7 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Use-After-Free in wacom_wac_pad_event due to dangling shared->touch_input pointer.
--

--- Patch [1]: [PATCH v7 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration ---
commit c2b94041c1337efa1a4a2cba0f9ae1c4dd7c4773
Author: Lee Jones <lee@kernel.org>

HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration

This commit moves the advertisement of the SW_MUTE_DEVICE capability to occur
before input device registration to comply with input subsystem guidelines. It
also modifies composite USB generic touch devices to advertise the mute
capability by default.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0eafa483b7f75..92b73ed0028f1 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2359,12 +2359,6 @@ 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);
> -	}
>  }

[Severity: Critical]
This is a pre-existing issue and was not introduced by this patch, but while
reviewing the shared touch input changes, I noticed a potential use-after-free
with wacom_wac->shared->touch_input.

In wacom_remove_shared_data(), when the touch interface unbinds, the devres
cleanup routine clears the shared->touch pointer but leaves
shared->touch_input dangling:

drivers/hid/wacom_sys.c:wacom_remove_shared_data() {
    ...
		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;
    ...
}

If the pad interface remains bound and a user triggers a touch mute event,
could wacom_wac_pad_event() access this freed input_dev?

drivers/hid/wacom_wac.c:wacom_wac_pad_event() {
    ...
	if ((equivalent_usage == WACOM_HID_WD_MUTE_DEVICE) ||
	   (equivalent_usage == WACOM_HID_WD_TOUCHONOFF)) {
		if (wacom_wac->shared->touch_input) {
			...
			input_report_switch(wacom_wac->shared->touch_input,
					    SW_MUTE_DEVICE, !(*is_touch_on));
			input_sync(wacom_wac->shared->touch_input);
		}
		return;
	}
    ...
}

Should wacom_remove_shared_data() also set shared->touch_input to NULL when
clearing shared->touch?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804103209.1496683-1-lee@kernel.org?part=1

Re: [PATCH v7 3/4] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] wacom_mode_change_work() accesses shared->pen and shared->touch locklessly outside of an RCU read-side critical section, leading to a Use-After-Free if a sibling device is removed.
- [High] wacom_remove_shared_data() calls kref_put() without holding wacom_udev_list_lock, creating a race condition where a concurrent probe can resurrect the object while it is being freed.
--

--- Patch [3]: [PATCH v7 3/4] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad ---
commit 2aec7c2beec55f0a42e84d2a747129503b809e66
Author: Lee Jones <lee@kernel.org>

HID: wacom: Fix Use-After-Free in wacom_bamboo_pad

This commit introduces RCU protection for pen and touch pointers in the
wacom_bamboo_pad driver. It adds RCU annotations and wraps lockless readers 
in rcu_read_lock() to prevent Use-After-Free bugs when sibling devices 
are removed.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index cf9c12f3cff53..05d27fcd311f8 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);
>  			}
>  		}
>  

[Severity: High]
This isn't a bug introduced by this patch, but does wacom_remove_shared_data()
have a race condition when calling kref_put()?

If kref_put(&data->kref, wacom_release_shared_data) is executed without holding
wacom_udev_list_lock, it drops the refcount to 0 and jumps to the release 
callback wacom_release_shared_data().

Before the release callback acquires wacom_udev_list_lock, a concurrent device
probe executing wacom_add_shared_data() could iterate over wacom_udev_list and
call kref_get() on the zero-refcount object.

Could a concurrent probe resurrect the object while it is being freed, leading
to a Use-After-Free?

[ ... ]

> @@ -2810,16 +2821,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: High]
This is a pre-existing issue, but does wacom_mode_change_work() have a
Use-After-Free risk because it accesses the pointers without RCU read-side
protection?

Since this workqueue does not hold rcu_read_lock(), a concurrent unbind
operation calling synchronize_rcu() can return immediately.

If a sibling device is unbound concurrently, could the pointer be freed while
this function continues to dereference it via hid_get_drvdata() and calls
blocking functions like hid_hw_stop()?

>  		wacom_release_resources(wacom1);
>  		hid_hw_stop(wacom1->hdev);
>  		wacom1->wacom_wac.has_mode_change = true;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804103209.1496683-1-lee@kernel.org?part=3

Re: [PATCH v7 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration

[Lee Jones]

Jason,

Can you confirm that you've seen this new version please?

[intentional top-post]


On Tue, 04 Aug 2026, Lee Jones 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, including composite USB generic touch devices.
> 
> Additionally, replace the lookup-dependent
> 'wacom_wac->shared->touch->product' references with 'hdev->product'
> inside wacom_setup_touch_input_capabilities() as 'hdev' is already
> available and represents the touch device itself.
> 
> 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
> 
>  drivers/hid/wacom_sys.c | 23 +++++++++++++++++------
>  drivers/hid/wacom_wac.c | 19 +++++++++++--------
>  2 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0eafa483b7f7..92b73ed0028f 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2359,12 +2359,6 @@ 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);
> -	}
>  }
>  
>  static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> @@ -2414,6 +2408,23 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
>  	wacom_retrieve_hid_descriptor(hdev, features);
>  	wacom_setup_device_quirks(wacom);
>  
> +	if (features->type == HID_GENERIC &&
> +	    (features->device_type & WACOM_DEVICETYPE_TOUCH)) {
> +		if (wacom->usbdev && wacom->usbdev->actconfig &&
> +		    wacom->usbdev->actconfig->desc.bNumInterfaces > 1) {
> +			/*
> +			 * Heuristic: Composite USB devices (like tablets with
> +			 * pen/pad + touch) likely have a touch mute switch.
> +			 * We flag it here to advertise the capability before
> +			 * registration. We also set is_soft_touch_switch to
> +			 * default touch to ON in case there is no physical
> +			 * switch.
> +			 */
> +			wacom_wac->has_mute_touch_switch = true;
> +			wacom_wac->is_soft_touch_switch = true;
> +		}
> +	}
> +
>  	if (features->device_type == WACOM_DEVICETYPE_NONE &&
>  	    features->type != WIRELESS) {
>  		error = features->type == HID_GENERIC ? -ENODEV : 0;
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index a29bf051ada7..afc82fcbb20b 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -3953,6 +3953,8 @@ 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))
> @@ -3963,9 +3965,12 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
>  	else
>  		__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
>  
> -	if (features->type == HID_GENERIC)
> +	if (features->type == HID_GENERIC) {
> +		if (wacom_wac->has_mute_touch_switch)
> +			input_set_capability(input_dev, EV_SW, SW_MUTE_DEVICE);
>  		/* setup has already been done */
>  		return 0;
> +	}
>  
>  	input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
>  	__set_bit(BTN_TOUCH, input_dev->keybit);
> @@ -3997,19 +4002,17 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
>  		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,
> @@ -4039,8 +4042,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) {
> +		if (hdev->product == 0x32C ||
> +		    hdev->product == 0xF6) {
>  			input_dev->evbit[0] |= BIT_MASK(EV_SW);
>  			__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
>  			wacom_wac->has_mute_touch_switch = true;
> -- 
> 2.55.0.571.g244d577d93-goog
> 

-- 
Lee Jones

Re: [PATCH v7 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration

[Jason Gerecke]

On Mon, Aug 10, 2026 at 3:57 AM Lee Jones <lee@kernel.org> wrote:
>
> Jason,
>
> Can you confirm that you've seen this new version please?
>
> [intentional top-post]
>
>
> On Tue, 04 Aug 2026, Lee Jones 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, including composite USB generic touch devices.
> >
> > Additionally, replace the lookup-dependent
> > 'wacom_wac->shared->touch->product' references with 'hdev->product'
> > inside wacom_setup_touch_input_capabilities() as 'hdev' is already
> > available and represents the touch device itself.
> >
> > 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
> >
> >  drivers/hid/wacom_sys.c | 23 +++++++++++++++++------
> >  drivers/hid/wacom_wac.c | 19 +++++++++++--------
> >  2 files changed, 28 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > index 0eafa483b7f7..92b73ed0028f 100644
> > --- a/drivers/hid/wacom_sys.c
> > +++ b/drivers/hid/wacom_sys.c
> > @@ -2359,12 +2359,6 @@ 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);
> > -     }
> >  }
> >
> >  static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> > @@ -2414,6 +2408,23 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> >       wacom_retrieve_hid_descriptor(hdev, features);
> >       wacom_setup_device_quirks(wacom);
> >
> > +     if (features->type == HID_GENERIC &&
> > +         (features->device_type & WACOM_DEVICETYPE_TOUCH)) {
> > +             if (wacom->usbdev && wacom->usbdev->actconfig &&
> > +                 wacom->usbdev->actconfig->desc.bNumInterfaces > 1) {
> > +                     /*
> > +                      * Heuristic: Composite USB devices (like tablets with
> > +                      * pen/pad + touch) likely have a touch mute switch.
> > +                      * We flag it here to advertise the capability before
> > +                      * registration. We also set is_soft_touch_switch to
> > +                      * default touch to ON in case there is no physical
> > +                      * switch.
> > +                      */
> > +                     wacom_wac->has_mute_touch_switch = true;
> > +                     wacom_wac->is_soft_touch_switch = true;
> > +             }
> > +     }
> > +

Apologies for the delay, Lee.

[ Begin ramble ]

This probably isn't a half-bad heuristic. I'm a bit hesitant to sign
off on it, mostly because I don't like the idea of the driver
advertising the existence of non-existent hardware.

My understanding is that it is theoretically allowable to change
capabilities post-registration (assuming we take due care to spinlock
relevant structures). The problem with doing so, however, is that if
userland queries the capabilities before we make the change, that it
will be stuck with an incorrect set. Software could theoretically
re-issue EVIOCGBIT ioctls at a later point in time to refresh
capabilities, but if we're honest that doesn't actually happen.

It feels like we're in a situation where we either:

A. Over-declare device capabilities on some devices (mostly older ones
that didn't have a touch switch)

-- or --

B. Have devices with under-declared capabilities if userland opens
them fast enough

The touch switch capability lets userland know to expect switch events
and to potentially offer some kind of software display of its state.
Its presence or absence is not critical since the switch state can
often be observed in other ways (physical hardware switch location) or
toggled back into a disabled state even if not immediately observable.
It's probably marginally better to err on providing state information
in situations where controls don't actually exist, but not by much.

[ End ramble ]

With all of that out of the way, I guess that I've convinced myself
that this is probably acceptable.

The only remaining nitpick I can identify looking at this again is
that the hunks that replace 'wacom_wac->shared->touch->product' with
'hdev->product' should really be in their own standalone cleanup patch
rather than in this (slightly-) behavior modifying one.

Address that one nitpick and I'll send out my reviewed-by for the two
new patches. Apologies again for my many delays... This should be the
last one :)

Jason (she/they)
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

> >       if (features->device_type == WACOM_DEVICETYPE_NONE &&
> >           features->type != WIRELESS) {
> >               error = features->type == HID_GENERIC ? -ENODEV : 0;
> > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> > index a29bf051ada7..afc82fcbb20b 100644
> > --- a/drivers/hid/wacom_wac.c
> > +++ b/drivers/hid/wacom_wac.c
> > @@ -3953,6 +3953,8 @@ 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))
> > @@ -3963,9 +3965,12 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
> >       else
> >               __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> >
> > -     if (features->type == HID_GENERIC)
> > +     if (features->type == HID_GENERIC) {
> > +             if (wacom_wac->has_mute_touch_switch)
> > +                     input_set_capability(input_dev, EV_SW, SW_MUTE_DEVICE);
> >               /* setup has already been done */
> >               return 0;
> > +     }
> >
> >       input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> >       __set_bit(BTN_TOUCH, input_dev->keybit);
> > @@ -3997,19 +4002,17 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
> >               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,
> > @@ -4039,8 +4042,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) {
> > +             if (hdev->product == 0x32C ||
> > +                 hdev->product == 0xF6) {
> >                       input_dev->evbit[0] |= BIT_MASK(EV_SW);
> >                       __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
> >                       wacom_wac->has_mute_touch_switch = true;
> > --
> > 2.55.0.571.g244d577d93-goog
> >
>
> --
> Lee Jones
>

Re: [PATCH v7 1/4] HID: wacom: Advertise SW_MUTE_DEVICE capability prior to registration

[Lee Jones]

On Tue, 11 Aug 2026, Jason Gerecke wrote:

> On Mon, Aug 10, 2026 at 3:57 AM Lee Jones <lee@kernel.org> wrote:
> >
> > Jason,
> >
> > Can you confirm that you've seen this new version please?
> >
> > [intentional top-post]
> >
> >
> > On Tue, 04 Aug 2026, Lee Jones 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, including composite USB generic touch devices.
> > >
> > > Additionally, replace the lookup-dependent
> > > 'wacom_wac->shared->touch->product' references with 'hdev->product'
> > > inside wacom_setup_touch_input_capabilities() as 'hdev' is already
> > > available and represents the touch device itself.
> > >
> > > 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
> > >
> > >  drivers/hid/wacom_sys.c | 23 +++++++++++++++++------
> > >  drivers/hid/wacom_wac.c | 19 +++++++++++--------
> > >  2 files changed, 28 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > > index 0eafa483b7f7..92b73ed0028f 100644
> > > --- a/drivers/hid/wacom_sys.c
> > > +++ b/drivers/hid/wacom_sys.c
> > > @@ -2359,12 +2359,6 @@ 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);
> > > -     }
> > >  }
> > >
> > >  static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> > > @@ -2414,6 +2408,23 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> > >       wacom_retrieve_hid_descriptor(hdev, features);
> > >       wacom_setup_device_quirks(wacom);
> > >
> > > +     if (features->type == HID_GENERIC &&
> > > +         (features->device_type & WACOM_DEVICETYPE_TOUCH)) {
> > > +             if (wacom->usbdev && wacom->usbdev->actconfig &&
> > > +                 wacom->usbdev->actconfig->desc.bNumInterfaces > 1) {
> > > +                     /*
> > > +                      * Heuristic: Composite USB devices (like tablets with
> > > +                      * pen/pad + touch) likely have a touch mute switch.
> > > +                      * We flag it here to advertise the capability before
> > > +                      * registration. We also set is_soft_touch_switch to
> > > +                      * default touch to ON in case there is no physical
> > > +                      * switch.
> > > +                      */
> > > +                     wacom_wac->has_mute_touch_switch = true;
> > > +                     wacom_wac->is_soft_touch_switch = true;
> > > +             }
> > > +     }
> > > +
> 
> Apologies for the delay, Lee.
> 
> [ Begin ramble ]
> 
> This probably isn't a half-bad heuristic. I'm a bit hesitant to sign
> off on it, mostly because I don't like the idea of the driver
> advertising the existence of non-existent hardware.
> 
> My understanding is that it is theoretically allowable to change
> capabilities post-registration (assuming we take due care to spinlock
> relevant structures). The problem with doing so, however, is that if
> userland queries the capabilities before we make the change, that it
> will be stuck with an incorrect set. Software could theoretically
> re-issue EVIOCGBIT ioctls at a later point in time to refresh
> capabilities, but if we're honest that doesn't actually happen.
> 
> It feels like we're in a situation where we either:
> 
> A. Over-declare device capabilities on some devices (mostly older ones
> that didn't have a touch switch)
> 
> -- or --
> 
> B. Have devices with under-declared capabilities if userland opens
> them fast enough
> 
> The touch switch capability lets userland know to expect switch events
> and to potentially offer some kind of software display of its state.
> Its presence or absence is not critical since the switch state can
> often be observed in other ways (physical hardware switch location) or
> toggled back into a disabled state even if not immediately observable.
> It's probably marginally better to err on providing state information
> in situations where controls don't actually exist, but not by much.
> 
> [ End ramble ]
> 
> With all of that out of the way, I guess that I've convinced myself
> that this is probably acceptable.
> 
> The only remaining nitpick I can identify looking at this again is
> that the hunks that replace 'wacom_wac->shared->touch->product' with
> 'hdev->product' should really be in their own standalone cleanup patch
> rather than in this (slightly-) behavior modifying one.
> 
> Address that one nitpick and I'll send out my reviewed-by for the two
> new patches. Apologies again for my many delays... This should be the
> last one :)

Okay, leave it with me.

-- 
Lee Jones