[PATCH 1/1] HID: wacom: Fix multiple Use-After-Free issues in shared state

[PATCH 1/1] HID: wacom: Fix multiple Use-After-Free issues in shared state

[Lee Jones]

The Wacom driver coordinates state between sibling interfaces of the same
physical device (like Pen, Touch, Pad) 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' or wacom_wac->shared->touch_input, leading to two Use-After-Free
vulnerabilities:

  1. dangling 'touch_input' dereferenced during touch switch sync.
  2. dangling 'data->dev' dereferenced during subsequent sibling probes.

Instead of adding complex pointer handover logic to keep 'data->dev'
updated (which has logic gaps with Pad siblings and introduces race
conditions), completely eliminate 'data->dev' pointer.

Redesign 'wacom_hdev_data' to store stable static copies of the required
representative attributes when it is first allocated:

  - Copy 'phys' path string (stored in data->phys) for stable path comparison.
  - Copy 'vendor' and 'product' IDs.
  - Copy and accumulate 'device_type' capabilities as siblings are probed.

Also explicitly clear 'touch_input = NULL' in wacom_remove_shared_data()
under wacom_udev_list_lock to safely avoid the touch_input UAF.

This resolves all vulnerabilities permanently at the design level without
complex pointer lifecycles or race-prone swaps on device removal.

Fixes: 471d17148c8b ("Input: wacom - move the USB (now hid) Wacom driver in drivers/hid")
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/hid/wacom_sys.c | 58 ++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 18 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a32320b351e3..9e4819c197c1 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -753,27 +753,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;
 
 	/*
@@ -781,11 +794,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;
 	}
 
@@ -798,7 +811,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;
 
 	/*
@@ -806,17 +819,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;
 
 	/*
@@ -832,7 +845,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;
 		}
@@ -840,7 +853,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;
 		}
@@ -871,10 +884,14 @@ 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)
+			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);
 
 		kref_put(&data->kref, wacom_release_shared_data);
 		wacom_wac->shared = NULL;
@@ -899,8 +916,13 @@ 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);
+	} else {
+		data->device_type |= wacom_wac->features.device_type;
 	}
 
 	mutex_unlock(&wacom_udev_list_lock);
-- 
2.54.0.746.g67dd491aae-goog

Re: [PATCH 1/1] HID: wacom: Fix multiple Use-After-Free issues in shared state

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fails to fully resolve the Use-After-Free vulnerability for `touch_input` because interrupt handlers access it locklessly, leading to a Time-of-Check to Time-of-Use (TOCTOU) race condition.
- [High] A race condition exists between `kref_put()` and concurrent `kref_get()` due to missing list-level locking during object removal.
- [High] Unprotected and temporally separated assignments of `shared->touch` and `shared->touch_input` during probe allow concurrent `uhid` devices to desynchronize the shared state.
--

commit 007cb667e9154320e49d7d42911e8193520da3fe
Author: Lee Jones <lee@kernel.org>

HID: wacom: Fix multiple Use-After-Free issues in shared state

This commit redesigns the wacom_hdev_data structure to store stable static
copies of required attributes instead of a fragile representative pointer.
It explicitly clears the touch_input pointer in wacom_remove_shared_data()
under the wacom_udev_list_lock to address use-after-free vulnerabilities.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index a32320b351e3e..9e4819c197c1b 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c

[ ... ]

> @@ -871,10 +884,14 @@ 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)
> +			wacom_wac->shared->touch_input = NULL;

[Severity: High]
This isn't a bug introduced by this patch, but does clearing this pointer under
the lock fully prevent use-after-free conditions?

Interrupt handlers read shared->touch_input locklessly (for example, in
wacom_wireless_irq()). An interrupt handler could check that
shared->touch_input is valid, but then get preempted.

Concurrently, wacom_remove_shared_data() clears the pointer here, and the
input device is subsequently unregistered and freed by devres.

When the interrupt handler resumes, could it pass the freed pointer to
input_report_switch(), causing a use-after-free? The lock protects writers,
but it appears readers remain vulnerable to this time-of-check to
time-of-use race.

> +		} else if (wacom_wac->shared->pen == wacom->hdev) {
>  			wacom_wac->shared->pen = NULL;
> +		}
> +		mutex_unlock(&wacom_udev_list_lock);
>  
>  		kref_put(&data->kref, wacom_release_shared_data);

[Severity: High]
This is a pre-existing issue, but can this kref_put() race with concurrent
list traversal?

It is called outside of wacom_udev_list_lock. If the reference count drops to 0,
another thread probing a sibling device could simultaneously acquire the list
lock in wacom_add_shared_data(), find the logically dead object before it is
removed from the list, and call kref_get().

This would attempt to increment a 0 refcount, returning a pointer to an object
that will unconditionally be freed as soon as the first thread acquires the
lock in wacom_release_shared_data(), leading to a use-after-free.

>  		wacom_wac->shared = NULL;

[ ... ]

> @@ -899,8 +916,13 @@ 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);
> +	} else {
> +		data->device_type |= wacom_wac->features.device_type;
>  	}
>  
>  	mutex_unlock(&wacom_udev_list_lock);

[Severity: High]
This is a pre-existing issue, but do the assignments to shared->touch and
shared->touch_input that follow this unlock need protection?

They are performed sequentially outside of any lock during device probe:

	if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
		wacom_wac->shared->touch = hdev;

If two virtual devices with the same physical path are created concurrently
(for example, via uhid), they will share the same wacom_hdev_data structure.

Could a race in the probe sequence result in shared->touch pointing to
device B, while shared->touch_input points to device A?

If device A is then disconnected, wacom_remove_shared_data() would see
shared->touch == wacom->hdev as false and skip clearing shared->touch_input.
Device A's input device would be freed, leaving a dangling pointer that could
be dereferenced by sibling devices.

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

Re: [PATCH 1/1] HID: wacom: Fix multiple Use-After-Free issues in shared state

[Dmitry Torokhov]

Hi Lee,

On Wed, May 27, 2026 at 03:07:30PM +0100, Lee Jones wrote:
> The Wacom driver coordinates state between sibling interfaces of the same
> physical device (like Pen, Touch, Pad) 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' or wacom_wac->shared->touch_input, leading to two Use-After-Free
> vulnerabilities:
> 
>   1. dangling 'touch_input' dereferenced during touch switch sync.
>   2. dangling 'data->dev' dereferenced during subsequent sibling probes.
> 
> Instead of adding complex pointer handover logic to keep 'data->dev'
> updated (which has logic gaps with Pad siblings and introduces race
> conditions), completely eliminate 'data->dev' pointer.
> 
> Redesign 'wacom_hdev_data' to store stable static copies of the required
> representative attributes when it is first allocated:
> 
>   - Copy 'phys' path string (stored in data->phys) for stable path comparison.
>   - Copy 'vendor' and 'product' IDs.

This I think makes sense.

>   - Copy and accumulate 'device_type' capabilities as siblings are probed.

This (accumulation) I unconvinced is safe. In any case I think it should
be a separate patch as it may change the behavior.

> 
> Also explicitly clear 'touch_input = NULL' in wacom_remove_shared_data()
> under wacom_udev_list_lock to safely avoid the touch_input UAF.

The fix is incomplete and should be split out. It is not enough to take
the lock, you need to make sure you are not racing with URB/IRQ
handling. Maybe RCU can help here.

> 
> This resolves all vulnerabilities permanently at the design level without
> complex pointer lifecycles or race-prone swaps on device removal.
> 
> Fixes: 471d17148c8b ("Input: wacom - move the USB (now hid) Wacom driver in drivers/hid")

This is not the commit that introduced this behavior IIRC.

Thanks.

-- 
Dmitry