Re: [PATCH] HID: synchronize input before cleaning up a failed probe

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free in hidraw_report_event() due to drivers calling hid_hw_stop() before hid_device_io_stop() on probe error paths.
--

commit 6c338032433c7e95cf76d26b6790e7b5f487a9b4
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>

HID: synchronize input before cleaning up a failed probe
    
hid_device_io_start() allows reports to run concurrently with probe. If
the probe subsequently fails, __hid_device_probe() releases driver
resources and clears hdev->driver without first excluding those report
callbacks.

Closes: https://syzkaller.appspot.com/bug?extid=9eebf5f6544c5e873858

> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 41a79e43c82b3..469b849b7a2a4 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2835,6 +2835,8 @@ static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv)
>  	 */
>  
>  	if (ret) {
> +		if (hdev->io_started)
> +			hid_device_io_stop(hdev);

[Severity: High]
This is a pre-existing issue, but does this fix cover all the cases where the
use-after-free can occur?

If drivers call hid_hw_stop(hdev) in their own probe error paths before
returning an error, they might bypass the core's new hid_device_io_stop()
cleanup.

hid_hw_stop() unconditionally calls hid_disconnect(), which frees the hidraw
object. If HID_QUIRK_ALWAYS_POLL is set (or for Bluetooth devices), polling
continues, and a concurrent report could enter hidraw_report_event() with a
dangling pointer before driver_input_lock is taken.

For example, this pattern appears in nintendo_hid_probe():

drivers/hid/hid-nintendo.c:nintendo_hid_probe() {
    ...
err_close:
	hid_hw_close(hdev);
err_stop:
	hid_hw_stop(hdev);
    ...
}

And similarly in corsairpsu_probe():

drivers/hwmon/corsair-psu.c:corsairpsu_probe() {
    ...
fail_and_close:
	hid_hw_close(hdev);
fail_and_stop:
	hid_hw_stop(hdev);
    ...
}

Could these drivers also trigger the use-after-free by tearing down the
hardware and freeing hidraw while IO is still started and driver_input_lock
is unheld?

>  		devres_release_group(&hdev->dev, hdev->devres_group_id);
>  		hid_close_report(hdev);
>  		hdev->driver = NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628005846.31248-1-alhouseenyousef@gmail.com?part=1