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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unbinding the primary interface leaves a dangling or unbound `data->dev` pointer in `wacom_udev_list`, causing a crash on subsequent device probes.
- [High] Lockless and un-synchronized dereference of `shared->pen` and `shared->touch` in `wacom_mode_change_work()` leads to a Use-After-Free.
--

--- Patch [3]: [PATCH v6 3/4] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad ---
commit bc217aafc6e3e6dc78c0e754ee61de0854395e8e
Author: Lee Jones <lee@kernel.org>
Date:   Thu Jul 16 09:07:14 2026 +0000

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

This patch resolves a use-after-free vulnerability when a sibling device
is removed while events are being processed in wacom_bamboo_pad_pen_event().
It introduces RCU protection for pen and touch pointers in the wacom_shared
struct to synchronize readers and writers safely.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index c6b37931d02d1..5578de80f4c80 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)

[Severity: High]
This isn't a bug introduced by this patch, but can unbinding the primary
interface leave a dangling pointer in wacom_udev_list, causing a crash on
subsequent device probes?

If a device with multiple interfaces is probed, and the first interface
allocates wacom_hdev_data, the second interface will increment its refcount.

If the first interface is unbound, its hdev is destroyed by the HID core, but
the shared data structure is kept alive by the second interface's refcount.
Does this leave a dangling pointer?

When a new device is probed, could wacom_get_hdev_data() iterate over
wacom_udev_list and call wacom_are_sibling(), resulting in accesses to the
freed hdev?

drivers/hid/wacom_sys.c:wacom_are_sibling() {
    ...
    if (features->oVid != HID_ANY_ID && sibling->vendor != oVid)
        ...
    if (!(sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
        ...
}

Could this lead to accessing a freed hdev or dereferencing an invalid offset
if hid_get_drvdata(sibling) returns NULL?

>  				    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));

[ ... ]

> @@ -2793,16 +2804,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 isn't a bug introduced by this patch, but does the lockless and
unsynchronized dereference of shared->pen and shared->touch in
wacom_mode_change_work() lead to a use-after-free?

The patch introduces rcu_access_pointer() here to avoid sparse warnings, but
reads the pointer using rcu_access_pointer() which is intended only for
testing for NULL, not for subsequent dereferencing outside an RCU critical
section.

Since wacom_mode_change_work() does not hold rcu_read_lock() nor acquire a
stable reference to the underlying hid_device, if the pen or touch device is
concurrently unbound, wacom_remove_shared_data() nullifies the pointers and
calls synchronize_rcu().

Because wacom_mode_change_work() does not hold the RCU lock,
synchronize_rcu() doesn't wait for it to finish.

Once the device is freed, won't wacom_mode_change_work() proceed to
dereference the dangling pointer in hid_get_drvdata(pen) and 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/20260716090726.2711293-1-lee@kernel.org?part=3