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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Lockless TOCTOU Use-After-Free of sibling devices in wacom_mode_change_work
--

commit bb8e808fb26bf4a0ca1a10604a506fb1d60c8173
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 in
wacom_intuos_pad() that could lead to a use-after-free. It introduces
RCU protection for the touch_input pointer, wrapping lockless readers
with rcu_read_lock() and writers with rcu_assign_pointer().

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 2220168bf1164..7ba5898265481 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();

[Severity: High]
This is a pre-existing issue, but is it possible for a sibling device to
still be used after this synchronize_rcu() call?

Looking at wacom_mode_change_work(), it reads shared->pen and shared->touch
locklessly:

drivers/hid/wacom_sys.c:wacom_mode_change_work() {
    ...
	if (shared->pen) {
		wacom1 = hid_get_drvdata(shared->pen);
		wacom_release_resources(wacom1);
		hid_hw_stop(wacom1->hdev);
    ...
}

If a context switch occurs right after shared->pen is evaluated as true,
could a sibling device be physically disconnected, triggering
wacom_remove_shared_data() to nullify shared->pen and free the object?

Because wacom_mode_change_work() does not hold an rcu_read_lock(), this
synchronize_rcu() wouldn't block it. When the workqueue resumes, could it
pass the now-freed pointer to hid_get_drvdata() and cause a use-after-free?

>  
>  		kref_put(&data->kref, wacom_release_shared_data);
>  		wacom_wac->shared = NULL;

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