Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
@ 2026-06-01 14:08 Lee Jones
  2026-06-01 14:08 ` [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Lee Jones @ 2026-06-01 14:08 UTC (permalink / raw)
  To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
	Dmitry Torokhov, Peter Hutterer, linux-input, linux-kernel

wacom_intuos_pad() accesses wacom->shared->touch_input locklessly inside the
interrupt handler context.  If the Touch sibling device is disconnected,
wacom_remove_shared_data() clears 'touch_input' outside any lock, creating a
Time-of-Check to Time-of-Use (TOCTOU) race condition where a preempted reader in
interrupt context dereferences the freed pointer, leading to a Use-After-Free.

Resolve this by introducing RCU protection for the touch_input pointer:

 - Annotate 'touch_input' in wacom_shared struct with __rcu
 - Wrap lockless readers in wacom_wac.c with rcu_read_lock() and rcu_dereference()
 - Update writers in wacom_sys.c using rcu_assign_pointer()
 - Call synchronize_rcu() in wacom_remove_shared_data() to ensure all active RCU
   readers have finished before the input device is freed

To ensure the devres release order is correct and wacom_remove_shared_data()
completes its RCU synchronization BEFORE the input device is freed, move the
wacom_add_shared_data() call to the very end of wacom_parse_and_register().
Since devres actions are executed in LIFO order, this guarantees the shared
reference is cleared and synchronized before the input device resource is destroyed.

Also wrap wacom_set_shared_values() and touch/pen assignments in
wacom_add_shared_data() inside the wacom_udev_list_lock to serialize concurrent
probe assignments, and verify that 'shared->touch == hdev' before setting
touch_input to prevent concurrent sibling probe state desynchronization.

Fixes: 961794a00eab ("Input: wacom - add reporting of SW_MUTE_DEVICE events")
Signed-off-by: Lee Jones <lee@kernel.org>
---

v1 -> v2: Split and use RCU as per Dmitry's review

 drivers/hid/wacom_sys.c | 49 +++++++++++++++++++++++++++--------------
 drivers/hid/wacom_wac.c | 17 +++++++++-----
 drivers/hid/wacom_wac.h |  2 +-
 3 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2220168bf116..26fce6d3198a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
 		data = container_of(wacom_wac->shared, struct wacom_hdev_data,
 				    shared);
 
-		if (wacom_wac->shared->touch == wacom->hdev)
+		mutex_lock(&wacom_udev_list_lock);
+		if (wacom_wac->shared->touch == wacom->hdev) {
 			wacom_wac->shared->touch = NULL;
-		else if (wacom_wac->shared->pen == wacom->hdev)
+			rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
+		} else if (wacom_wac->shared->pen == wacom->hdev) {
 			wacom_wac->shared->pen = NULL;
+		}
+		mutex_unlock(&wacom_udev_list_lock);
+
+		synchronize_rcu();
 
 		kref_put(&data->kref, wacom_release_shared_data);
 		wacom_wac->shared = NULL;
@@ -909,6 +915,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)
 		list_add_tail(&data->list, &wacom_udev_list);
 	}
 
+	if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
+		data->shared.touch = hdev;
+	else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
+		data->shared.pen = hdev;
+
 	mutex_unlock(&wacom_udev_list_lock);
 
 	wacom_wac->shared = &data->shared;
@@ -917,11 +928,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
 	if (retval)
 		return retval;
 
-	if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
-		wacom_wac->shared->touch = hdev;
-	else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
-		wacom_wac->shared->pen = hdev;
-
 	return retval;
 }
 
@@ -2345,9 +2351,15 @@ static void wacom_release_resources(struct wacom *wacom)
 
 static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
 {
+	struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+
+	mutex_lock(&wacom_udev_list_lock);
+
 	if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
-		wacom_wac->shared->type = wacom_wac->features.type;
-		wacom_wac->shared->touch_input = wacom_wac->touch_input;
+		if (wacom_wac->shared->touch == wacom->hdev) {
+			wacom_wac->shared->type = wacom_wac->features.type;
+			rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
+		}
 	}
 
 	if (wacom_wac->has_mute_touch_switch) {
@@ -2362,11 +2374,14 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
 	}
 
 	if (wacom_wac->shared->has_mute_touch_switch &&
-	    wacom_wac->shared->touch_input) {
-		set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
-		input_set_capability(wacom_wac->shared->touch_input, EV_SW,
-				     SW_MUTE_DEVICE);
+	    rcu_access_pointer(wacom_wac->shared->touch_input)) {
+		struct input_dev *touch_input = rcu_dereference_protected(wacom_wac->shared->touch_input,
+									lockdep_is_held(&wacom_udev_list_lock));
+		set_bit(EV_SW, touch_input->evbit);
+		input_set_capability(touch_input, EV_SW, SW_MUTE_DEVICE);
 	}
+
+	mutex_unlock(&wacom_udev_list_lock);
 }
 
 static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
@@ -2442,10 +2457,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 		goto fail;
 	}
 
-	error = wacom_add_shared_data(hdev);
-	if (error)
-		goto fail;
-
 	error = wacom_setup_inputs(wacom);
 	if (error)
 		goto fail;
@@ -2496,6 +2507,10 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 		}
 	}
 
+	error = wacom_add_shared_data(hdev);
+	if (error)
+		goto fail_quirks;
+
 	wacom_set_shared_values(wacom_wac);
 	devres_close_group(&hdev->dev, wacom);
 
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..c7cf1aaab903 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -650,11 +650,18 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
 	input_report_key(input, KEY_CONTROLPANEL, menu);
 	input_report_key(input, KEY_INFO, info);
 
-	if (wacom->shared && wacom->shared->touch_input) {
-		input_report_switch(wacom->shared->touch_input,
-				    SW_MUTE_DEVICE,
-				    !wacom->shared->is_touch_on);
-		input_sync(wacom->shared->touch_input);
+	if (wacom->shared) {
+		struct input_dev *touch_input;
+
+		rcu_read_lock();
+		touch_input = rcu_dereference(wacom->shared->touch_input);
+		if (touch_input) {
+			input_report_switch(touch_input,
+					    SW_MUTE_DEVICE,
+					    !wacom->shared->is_touch_on);
+			input_sync(touch_input);
+		}
+		rcu_read_unlock();
 	}
 
 	input_report_abs(input, ABS_RX, strip1);
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 126bec6e5c0c..a8bbba4a6f37 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -285,7 +285,7 @@ struct wacom_shared {
 	/* for wireless device to access USB interfaces */
 	unsigned touch_max;
 	int type;
-	struct input_dev *touch_input;
+	struct input_dev __rcu *touch_input;
 	struct hid_device *pen;
 	struct hid_device *touch;
 	bool has_mute_touch_switch;
-- 
2.54.0.823.g6e5bcc1fc9-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle
  2026-06-01 14:08 [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
@ 2026-06-01 14:08 ` Lee Jones
  2026-06-01 14:26 ` [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad sashiko-bot
  2026-06-01 14:48 ` Markus Elfring
  2 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2026-06-01 14:08 UTC (permalink / raw)
  To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
	Dmitry Torokhov, Peter Hutterer, linux-input, linux-kernel

The Wacom driver coordinates state between sibling interfaces of the same
physical device using a shared structure 'wacom_shared' inside
'wacom_hdev_data'.  The driver kept a volatile representative pointer
'data->dev' pointing to a sibling 'hid_device' for physical path comparisons
during sibling matching.

This pointer management is fragile.  When the representative device is
disconnected, wacom_remove_shared_data() failed to clear/update
'data->dev', leading to a Use-After-Free vulnerability when subsequent
sibling probes dereference the dangling 'data->dev' pointer.

Resolve this issue by redesigning the sibling data lifecycle:

 - Eliminate the volatile 'data->dev' representative pointer completely
 - Redesign 'wacom_hdev_data' to store stable static copies of the required
   attributes upon first allocation: 'phys' path string, 'vendor', 'product
   IDs and the sibling's 'device_type'
 - Use these static attributes for stable sibling matching in
   wacom_are_sibling() and wacom_get_hdev_data()

This ensures sibling matching remains safe and stable even if individual
siblings are dynamically added or removed.

To secure the lifecycle against concurrent probe/disconnect races:

- Switch kref_put() to kref_put_mutex() in wacom_remove_shared_data() to
  serialize refcount drops with list traversal and lookup
- Modify wacom_release_shared_data() to assume the list lock is already held

Also, do not accumulate the 'device_type' capability flag during subsequent
sibling probes.  Keeping only the first probed sibling's device_type exactly
preserves the original sibling matching behavior without introducing side
effects.

Fixes: 4492efffffeb ("Input: wacom - share pen info with touch of the same ID")
Signed-off-by: Lee Jones <lee@kernel.org>
---

v1 -> v2: Split and use RCU as per Dmitry's review

drivers/hid/wacom_sys.c | 63 +++++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 24 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 26fce6d3198a..7cd0b0ff3128 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -759,27 +759,40 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
 struct wacom_hdev_data {
 	struct list_head list;
 	struct kref kref;
-	struct hid_device *dev;
+	char phys[64];
+	__u32 vendor;
+	__u32 product;
+	__u32 device_type;
 	struct wacom_shared shared;
 };
 
+static bool wacom_compare_device_paths(struct hid_device *hdev_a,
+				       const char *phys_b, char separator)
+{
+	int n1 = strrchr(hdev_a->phys, separator) - hdev_a->phys;
+	int n2 = strrchr(phys_b, separator) - phys_b;
+
+	if (n1 != n2 || n1 <= 0 || n2 <= 0)
+		return false;
+
+	return !strncmp(hdev_a->phys, phys_b, n1);
+}
+
 static LIST_HEAD(wacom_udev_list);
 static DEFINE_MUTEX(wacom_udev_list_lock);
 
 static bool wacom_are_sibling(struct hid_device *hdev,
-		struct hid_device *sibling)
+		struct wacom_hdev_data *data)
 {
 	struct wacom *wacom = hid_get_drvdata(hdev);
 	struct wacom_features *features = &wacom->wacom_wac.features;
-	struct wacom *sibling_wacom = hid_get_drvdata(sibling);
-	struct wacom_features *sibling_features = &sibling_wacom->wacom_wac.features;
 	__u32 oVid = features->oVid ? features->oVid : hdev->vendor;
 	__u32 oPid = features->oPid ? features->oPid : hdev->product;
 
 	/* The defined oVid/oPid must match that of the sibling */
-	if (features->oVid != HID_ANY_ID && sibling->vendor != oVid)
+	if (features->oVid != HID_ANY_ID && data->vendor != oVid)
 		return false;
-	if (features->oPid != HID_ANY_ID && sibling->product != oPid)
+	if (features->oPid != HID_ANY_ID && data->product != oPid)
 		return false;
 
 	/*
@@ -787,11 +800,11 @@ static bool wacom_are_sibling(struct hid_device *hdev,
 	 * device path, while those with different VID/PID must share
 	 * the same physical parent device path.
 	 */
-	if (hdev->vendor == sibling->vendor && hdev->product == sibling->product) {
-		if (!hid_compare_device_paths(hdev, sibling, '/'))
+	if (hdev->vendor == data->vendor && hdev->product == data->product) {
+		if (!wacom_compare_device_paths(hdev, data->phys, '/'))
 			return false;
 	} else {
-		if (!hid_compare_device_paths(hdev, sibling, '.'))
+		if (!wacom_compare_device_paths(hdev, data->phys, '.'))
 			return false;
 	}
 
@@ -804,7 +817,7 @@ static bool wacom_are_sibling(struct hid_device *hdev,
 	 * devices.
 	 */
 	if ((features->device_type & WACOM_DEVICETYPE_DIRECT) &&
-	    !(sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+	    !(data->device_type & WACOM_DEVICETYPE_DIRECT))
 		return false;
 
 	/*
@@ -812,17 +825,17 @@ static bool wacom_are_sibling(struct hid_device *hdev,
 	 * devices.
 	 */
 	if (!(features->device_type & WACOM_DEVICETYPE_DIRECT) &&
-	    (sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+	    (data->device_type & WACOM_DEVICETYPE_DIRECT))
 		return false;
 
 	/* Pen devices may only be siblings of touch devices */
 	if ((features->device_type & WACOM_DEVICETYPE_PEN) &&
-	    !(sibling_features->device_type & WACOM_DEVICETYPE_TOUCH))
+	    !(data->device_type & WACOM_DEVICETYPE_TOUCH))
 		return false;
 
 	/* Touch devices may only be siblings of pen devices */
 	if ((features->device_type & WACOM_DEVICETYPE_TOUCH) &&
-	    !(sibling_features->device_type & WACOM_DEVICETYPE_PEN))
+	    !(data->device_type & WACOM_DEVICETYPE_PEN))
 		return false;
 
 	/*
@@ -838,7 +851,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
 
 	/* Try to find an already-probed interface from the same device */
 	list_for_each_entry(data, &wacom_udev_list, list) {
-		if (hid_compare_device_paths(hdev, data->dev, '/')) {
+		if (wacom_compare_device_paths(hdev, data->phys, '/')) {
 			kref_get(&data->kref);
 			return data;
 		}
@@ -846,7 +859,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
 
 	/* Fallback to finding devices that appear to be "siblings" */
 	list_for_each_entry(data, &wacom_udev_list, list) {
-		if (wacom_are_sibling(hdev, data->dev)) {
+		if (wacom_are_sibling(hdev, data)) {
 			kref_get(&data->kref);
 			return data;
 		}
@@ -860,35 +873,34 @@ static void wacom_release_shared_data(struct kref *kref)
 	struct wacom_hdev_data *data =
 		container_of(kref, struct wacom_hdev_data, kref);
 
-	mutex_lock(&wacom_udev_list_lock);
 	list_del(&data->list);
-	mutex_unlock(&wacom_udev_list_lock);
-
 	kfree(data);
 }
 
 static void wacom_remove_shared_data(void *res)
 {
-	struct wacom *wacom = res;
+	struct wacom *res_wacom = res;
 	struct wacom_hdev_data *data;
-	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct wacom_wac *wacom_wac = &res_wacom->wacom_wac;
 
 	if (wacom_wac->shared) {
 		data = container_of(wacom_wac->shared, struct wacom_hdev_data,
 				    shared);
 
 		mutex_lock(&wacom_udev_list_lock);
-		if (wacom_wac->shared->touch == wacom->hdev) {
+		if (wacom_wac->shared->touch == res_wacom->hdev) {
 			wacom_wac->shared->touch = NULL;
 			rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
-		} else if (wacom_wac->shared->pen == wacom->hdev) {
+		} else if (wacom_wac->shared->pen == res_wacom->hdev) {
 			wacom_wac->shared->pen = NULL;
 		}
 		mutex_unlock(&wacom_udev_list_lock);
 
 		synchronize_rcu();
 
-		kref_put(&data->kref, wacom_release_shared_data);
+		if (kref_put_mutex(&data->kref, wacom_release_shared_data, &wacom_udev_list_lock))
+			mutex_unlock(&wacom_udev_list_lock);
+
 		wacom_wac->shared = NULL;
 	}
 }
@@ -911,7 +923,10 @@ static int wacom_add_shared_data(struct hid_device *hdev)
 		}
 
 		kref_init(&data->kref);
-		data->dev = hdev;
+		strncpy(data->phys, hdev->phys, sizeof(data->phys) - 1);
+		data->vendor = hdev->vendor;
+		data->product = hdev->product;
+		data->device_type = wacom_wac->features.device_type;
 		list_add_tail(&data->list, &wacom_udev_list);
 	}
 
-- 
2.54.0.823.g6e5bcc1fc9-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
  2026-06-01 14:08 [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
  2026-06-01 14:08 ` [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
@ 2026-06-01 14:26 ` sashiko-bot
  2026-06-01 14:48 ` Markus Elfring
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-01 14:26 UTC (permalink / raw)
  To: Lee Jones; +Cc: dmitry.torokhov, linux-input

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

New issues:
- [Critical] Moving wacom_add_shared_data() to the end of wacom_parse_and_register() causes an unconditional NULL pointer dereference during device probe.
- [High] Moving wacom_add_shared_data() to the end of the probe function creates a window where wacom_wac->shared is NULL while hardware interrupts are active, leading to an asynchronous NULL pointer dereference.

Pre-existing issues:
- [High] The patch incompletely fixes a Time-of-Check to Time-of-Use (TOCTOU) race condition. It successfully converts wacom_intuos_pad() to use RCU, but leaves identical lockless accesses unprotected in wacom_wac_pad_event(), wacom_wireless_irq(), and wacom_status_irq().
- [High] Modifying input capabilities on the sibling touch device after it has already been registered with the input core.
--

commit 5bec6e8c12bc84f46d120679cc4bef944e215dc2
Author: Lee Jones <lee@kernel.org>

HID: wacom: Fix Use-After-Free in wacom_intuos_pad
    
This commit resolves a Time-of-Check to Time-of-Use race condition by
introducing RCU protection for the touch_input pointer. It also delays the
call to wacom_add_shared_data() to ensure correct devres release order.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c

[ ... ]

> @@ -2362,11 +2374,14 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
>  	}
>  
>  	if (wacom_wac->shared->has_mute_touch_switch &&
> -	    wacom_wac->shared->touch_input) {
> -		set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
> -		input_set_capability(wacom_wac->shared->touch_input, EV_SW,
> -				     SW_MUTE_DEVICE);
> +	    rcu_access_pointer(wacom_wac->shared->touch_input)) {
> +		struct input_dev *touch_input = rcu_dereference_protected(wacom_wac->shared->touch_input,
> +									lockdep_is_held(&wacom_udev_list_lock));
> +		set_bit(EV_SW, touch_input->evbit);
> +		input_set_capability(touch_input, EV_SW, SW_MUTE_DEVICE);
>  	}
> +
> +	mutex_unlock(&wacom_udev_list_lock);
>  }

[Severity: High]
This is a pre-existing issue, but does this capability modification happen
too late?

In wacom_set_shared_values(), input capabilities are modified on the sibling
touch device. Because the Pen interface probes after the Touch interface,
this occurs on a sibling's touch_input device that was already fully probed
and registered.

The input subsystem requires that all capabilities must be fully initialized
before calling input_register_device(). Modifying capabilities
post-registration means the ADD uevent has already been emitted, and
userspace components miss the capability changes and might fail to process
events like SW_MUTE_DEVICE.

[ ... ]

> @@ -2442,10 +2457,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
>  		goto fail;
>  	}
>  
> -	error = wacom_add_shared_data(hdev);
> -	if (error)
> -		goto fail;
> -
>  	error = wacom_setup_inputs(wacom);
>  	if (error)
>  		goto fail;

[Severity: Critical]
Does this change introduce a regression during device probe?

By removing wacom_add_shared_data() from this earlier location,
wacom_wac->shared is left uninitialized (NULL). However, wacom_setup_inputs()
is called immediately after:

wacom_setup_inputs()
   wacom_setup_touch_input_capabilities()
       if (wacom_wac->shared->touch->product == 0x361) {

Because wacom_wac->shared is unconditionally dereferenced for some devices
like INTUOSP2_BT and WACOM_27QHDT inside wacom_setup_touch_input_capabilities(),
could this cause a NULL pointer dereference during initialization?

[ ... ]

> @@ -2496,6 +2507,10 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
>  		}
>  	}
>  
> +	error = wacom_add_shared_data(hdev);
> +	if (error)
> +		goto fail_quirks;
> +
>  	wacom_set_shared_values(wacom_wac);
>  	devres_close_group(&hdev->dev, wacom);

[Severity: High]
Is there a race condition here since wacom_add_shared_data() is now called
at the very end of the probe function?

Before reaching this point, wacom_parse_and_register() has already called
hid_hw_start() and wacom_register_inputs(). hid_hw_start() enables incoming
HID reports and wacom_register_inputs() exposes the device to userspace.

If a hardware interrupt fires right after hid_hw_start(), the interrupt
handlers can execute while wacom_wac->shared is still NULL:

wacom_bpt_pen()
    wacom->shared->stylus_in_proximity = true;

Could this cause an asynchronous NULL pointer dereference if a report
arrives before the shared data is allocated?

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c

[ ... ]

> @@ -650,11 +650,18 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
>  	input_report_key(input, KEY_CONTROLPANEL, menu);
>  	input_report_key(input, KEY_INFO, info);
>  
> -	if (wacom->shared && wacom->shared->touch_input) {
> -		input_report_switch(wacom->shared->touch_input,
> -				    SW_MUTE_DEVICE,
> -				    !wacom->shared->is_touch_on);
> -		input_sync(wacom->shared->touch_input);
> +	if (wacom->shared) {
> +		struct input_dev *touch_input;
> +
> +		rcu_read_lock();
> +		touch_input = rcu_dereference(wacom->shared->touch_input);
> +		if (touch_input) {
> +			input_report_switch(touch_input,
> +					    SW_MUTE_DEVICE,
> +					    !wacom->shared->is_touch_on);
> +			input_sync(touch_input);
> +		}
> +		rcu_read_unlock();
>  	}

[Severity: High]
This is a pre-existing issue, but did this patch miss a few other places
where touch_input is accessed locklessly?

This update successfully converts wacom_intuos_pad() to use RCU, but there
are other HID event handlers in wacom_wac.c that still directly dereference
wacom_wac->shared->touch_input without holding rcu_read_lock().

For example, in wacom_wac_pad_event():

    if (wacom_wac->shared->touch_input) {
        ...
        input_report_switch(wacom_wac->shared->touch_input, ...);

Similar direct accesses happen in wacom_wireless_irq() and
wacom_status_irq(). Because these readers do not establish an RCU read-side
critical section, synchronize_rcu() in wacom_remove_shared_data() will not
wait for them.

If the sibling touch device is unbound, could the input_dev be freed while
these handlers are still running, resulting in the same use-after-free issue?
Direct access to an __rcu pointer may also trigger Sparse warnings.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
  2026-06-01 14:08 [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
  2026-06-01 14:08 ` [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
  2026-06-01 14:26 ` [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad sashiko-bot
@ 2026-06-01 14:48 ` Markus Elfring
  2 siblings, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2026-06-01 14:48 UTC (permalink / raw)
  To: Lee Jones, linux-input, Benjamin Tissoires, Dmitry Torokhov,
	Jason Gerecke, Jiri Kosina, Peter Hutterer, Ping Cheng
  Cc: LKML> +++ b/drivers/hid/wacom_sys.c
> @@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
>  		data = container_of(wacom_wac->shared, struct wacom_hdev_data,
>  				    shared);
>  
> -		if (wacom_wac->shared->touch == wacom->hdev)
> +		mutex_lock(&wacom_udev_list_lock);
> +		if (wacom_wac->shared->touch == wacom->hdev) {
>  			wacom_wac->shared->touch = NULL;
> +		}
> +		mutex_unlock(&wacom_udev_list_lock);
> +
> +		synchronize_rcu();
>  
>  		kref_put(&data->kref, wacom_release_shared_data);
…

Under which circumstances would you dare to apply another lock guard?
https://elixir.bootlin.com/linux/v7.1-rc5/source/include/linux/mutex.h#L253

Regards,
Markus

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-01 14:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-01 14:08 [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad Lee Jones
2026-06-01 14:08 ` [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle Lee Jones
2026-06-01 14:26 ` [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad sashiko-bot
2026-06-01 14:48 ` Markus Elfring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox