Re: [PATCH v6 02/13] HID: asus: reinitialize the device after exiting a sleep state

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incorrect teardown order in remove and probe error paths.
--

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

HID: asus: reinitialize the device after exiting a sleep state
    
The ROG ally needs to have the EC string sent back after resuming from
s2idle since the USB device can be turned completely off by the firmware
when mcu_powersave firmware-attribute is set to 1.
    
This may also be true for other laptops and certain features might stop
working after the device exit from sleep.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index bd46aba6622a1..2125680c7effc 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c

[ ... ]

> @@ -1493,16 +1518,8 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		return ret;
>  	}
>  
> -	if (!drvdata->tp) {
> -		for (int r = 0; r < ARRAY_SIZE(asus_report_id_init); r++) {
> -			if (asus_has_report_id(hdev, asus_report_id_init[r])) {
> -				ret = asus_kbd_init(hdev, asus_report_id_init[r]);
> -				if (ret < 0)
> -					hid_warn(hdev, "Failed to initialize 0x%x: %d.\n",
> -						 asus_report_id_init[r], ret);
> -			}
> -		}
> -	}
> +	if (!drvdata->tp)
> +		asus_initialize_reports(hdev);
>  
>  	/* Laptops keyboard backlight is always at 0x5a */
>  	if (is_vendor && (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) &&

[Severity: High]
This isn't a bug introduced by this patch, but I noticed an incorrect
teardown order in the error path of asus_probe() and in asus_remove():

drivers/hid/hid-asus.c:asus_remove() {
    ...
	asus_worker_stop(drvdata->worker);
	hid_hw_stop(hdev);
}

Does this violate the HID core lifecycle rules? It appears the custom worker
is cancelled before the hardware device is stopped.

The subsystem guidelines mandate calling hid_hw_stop() before cancelling
workers to prevent new work from being queued by userspace or the hardware
after teardown has begun.

While there might be a custom worker flag to discard new work items during
teardown, does relying on custom synchronization rather than the correct core
teardown ordering risk a use-after-free during driver unbind or probe failure?

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