Re: [PATCH 3/3] HID: asus: add support for xgm led

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid: asus: I/O on stopped device in probe error path
--

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

HID: asus: add support for xgm led

This commit adds an LED class device interface to control the brightness
of the red LEDs on Asus XG mobile stations. It registers the LED using
the managed devm_led_classdev_register API and hooks up a callback to
set the brightness via HID reports.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index e6218abdac49c..825833f6c2c4d 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c

[ ... ]

> @@ -1545,6 +1616,16 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	if (!drvdata->tp)
>  		asus_initialize_reports(hdev);
>  
> +	if (asus_has_report_id(hdev, FEATURE_KBD_REPORT_ID) &&
> +	    ((hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2022) ||
> +	     (hdev->product == USB_DEVICE_ID_ASUSTEK_XGM_2023))) {
> +		ret = asus_xgm_init(hdev, drvdata);
> +		if (ret) {
> +			hid_err(hdev, "Failed to initialize xg mobile: %d\n", ret);
> +			goto err_stop_hw;
> +		}
> +	}
> +

[Severity: High]
Does the err_stop_hw path in asus_probe() need to manually unregister the
xgm_led before stopping the hardware?

If probe fails after asus_xgm_init() successfully registers the LED (for
example, if asus_start_multitouch() fails later in the probe function),
the execution jumps to err_stop_hw:

err_stop_hw:
	if (drvdata->listener.brightness_set)
		asus_hid_unregister_listener(&drvdata->listener);

	asus_worker_stop(drvdata->worker);
	hid_hw_stop(hdev);
	return ret;

Since the LED was registered with devm_led_classdev_register(), devres
will automatically unregister it when asus_probe() returns. However,
unregistering an LED classdev natively turns it off, invoking the
blocking asus_xgm_led_set() callback.

Because hid_hw_stop() has already shut down the hardware in this path,
asus_xgm_led_set() will call hid_hw_raw_request() on a stopped (and
potentially partially freed) HID transport. Depending on the lower-level
driver, this can lead to a use-after-free or a kernel crash.

I see this is handled safely in asus_remove() by explicitly unregistering
the LED before hid_hw_stop() is called. Could we do the same in the probe
error path?

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