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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Tue, Jun 09, 2026 at 01:13:37PM +0100, Lee Jones wrote:
> wacom_intuos_pad() accesses wacom->shared->touch_input locklessly
> inside the interrupt handler context.  If the Touch sibling device
> is disconnected, wacom_remove_shared_data() clears 'touch_input'
> outside any lock, creating a Time-of-Check to Time-of-Use (TOCTOU)
> race condition where a preempted reader in interrupt context
> dereferences the freed pointer, leading to a Use-After-Free.
> 
> Resolve this by introducing RCU protection for the touch_input
> pointer:
> 
>  - Annotate 'touch_input' in wacom_shared struct with __rcu
>  - Wrap all lockless readers in wacom_wac.c with rcu_read_lock() and
>    rcu_dereference() using a unified wacom_report_touch_mute()
>    helper
>  - Update writers in wacom_sys.c using rcu_assign_pointer()
>  - Call synchronize_rcu() in wacom_remove_shared_data() to ensure
>    all active RCU readers have finished before the input device is
>    freed
> 
> Also wrap wacom_set_shared_values() and touch/pen assignments in
> wacom_add_shared_data() inside the wacom_udev_list_lock to serialize
> concurrent probe assignments, and verify that 'shared->touch == hdev'
> before setting touch_input to prevent concurrent sibling probe state
> desynchronization.
> 
> Finally, advertise the SW_MUTE_DEVICE capability on Touch input
> devices prior to registration in wacom_setup_touch_input_capabilities()
> to prevent invalid post-registration capability modifications.
> 
> 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
> 
>  drivers/hid/wacom_sys.c | 41 ++++++++++++++----------
>  drivers/hid/wacom_wac.c | 70 ++++++++++++++++++++++-------------------
>  drivers/hid/wacom_wac.h |  2 +-
>  3 files changed, 63 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 2220168bf116..7ba589826548 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
>  		data = container_of(wacom_wac->shared, struct wacom_hdev_data,
>  				    shared);
>  
> -		if (wacom_wac->shared->touch == wacom->hdev)
> -			wacom_wac->shared->touch = NULL;
> -		else if (wacom_wac->shared->pen == wacom->hdev)
> -			wacom_wac->shared->pen = NULL;
> +		scoped_guard(mutex, &wacom_udev_list_lock) {
> +			if (wacom_wac->shared->touch == wacom->hdev) {
> +				wacom_wac->shared->touch = NULL;
> +				rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
> +			} else if (wacom_wac->shared->pen == wacom->hdev) {
> +				wacom_wac->shared->pen = NULL;
> +			}
> +		}
> +
> +		synchronize_rcu();
>  
>  		kref_put(&data->kref, wacom_release_shared_data);
>  		wacom_wac->shared = NULL;
> @@ -909,6 +915,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)
>  		list_add_tail(&data->list, &wacom_udev_list);
>  	}
>  
> +	if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
> +		data->shared.touch = hdev;
> +	else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
> +		data->shared.pen = hdev;
> +
>  	mutex_unlock(&wacom_udev_list_lock);
>  
>  	wacom_wac->shared = &data->shared;
> @@ -917,11 +928,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
>  	if (retval)
>  		return retval;
>  
> -	if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
> -		wacom_wac->shared->touch = hdev;
> -	else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
> -		wacom_wac->shared->pen = hdev;
> -
>  	return retval;
>  }
>  
> @@ -2345,9 +2351,15 @@ static void wacom_release_resources(struct wacom *wacom)
>  
>  static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
>  {
> +	struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
> +
> +	mutex_lock(&wacom_udev_list_lock);

Why not

	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) {
> @@ -2361,12 +2373,7 @@ 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);
> -	}
> +	mutex_unlock(&wacom_udev_list_lock);
>  }
>  
>  static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625..495960227b8d 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -510,6 +510,22 @@ 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;
> +
> +	if (!wacom_wac->shared)
> +		return;

Can this happen? I think callers already check this or simply
dereference.

> +
> +	rcu_read_lock();

	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);
> +	}
> +	rcu_read_unlock();
> +}
> +

Thanks.

-- 
Dmitry