Re: [PATCH v4 03/13] HID: asus: add support for ROG Ally handhelds

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Devres LIFO ordering violation causing a use-after-free during device removal.
--

commit 6c026e410beeb5bf25ebf0d772fc59dc024dc875
Author: Denis Benato <denis.benato@linux.dev>

    HID: asus: add support for ROG Ally handhelds

    Implement the core functionality for ASUS handhelds, extending the functionality
    of hid-asus if the driver is enabled. This initializes the device, implements 
    Ally X input mapping, and emits a single proper event for AC button long press.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> +static void ally_x_input_close(struct input_dev *dev)
> +{
> +	hid_hw_close(input_get_drvdata(dev));
> +}
> +
> +static struct input_dev *ally_x_alloc_input_dev(struct hid_device *hdev)
> +{
> +	struct input_dev *input_dev = devm_input_allocate_device(&hdev->dev);
[ ... ]
> +	input_dev->open = ally_x_input_open;
> +	input_dev->close = ally_x_input_close;
> +
> +	return input_dev;
> +}
[ ... ]
> @@ -1570,6 +2481,9 @@ static void asus_remove(struct hid_device *hdev)
>  	if (drvdata->listener.brightness_set)
>  		asus_hid_unregister_listener(&drvdata->listener);
>  
> +	if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD)
> +		hid_asus_ally_remove(hdev, drvdata->rog_ally);
> +
>  	asus_worker_stop(drvdata->worker);
>  	hid_hw_stop(hdev);
>  }

[Severity: High]
Does this manual call to hid_hw_stop() conflict with the devres-managed input 
device teardown?

Since ally_x_alloc_input_dev() allocates the input device using 
devm_input_allocate_device(), its unregistration is deferred until after 
this remove() callback finishes. 

If userspace has an open file descriptor on the gamepad device when the 
device is unplugged or unbound, the devres-triggered input_unregister_device() 
will call the input device's .close callback, which was set to 
ally_x_input_close():

ally_x_input_close()
    hid_hw_close(input_get_drvdata(dev));

Because hid_hw_stop() has already been executed here in asus_remove(), the 
transport resources have been destroyed. When ally_x_input_close() drops into 
the transport driver via hid_hw_close(), it will try to access those freed 
resources, leading to a use-after-free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830234320.114356-1-denis.benato@linux.dev?part=3