Re: [PATCH 3/4] hwmon: (nzxt-smart2) Stop device IO before calling hid_hw_stop

[sashiko-bot@kernel.org]

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

New issues:
- [Low] Unconditional call to `hid_device_io_stop()` on the error path triggers a spurious kernel warning when `hid_hw_open()` fails.

Pre-existing issues:
- [Critical] The same Use-After-Free (UAF) vulnerability exists in other HID hwmon drivers (nzxt-kraken3.c and corsair-psu.c) and was missed by this patch series.
- [High] Device removal can hang indefinitely if a user process is blocked reading sysfs attributes.
- [High] The return value of init_device() is silently ignored during probe, leading to permanent hangs on sysfs reads.
--

--- Patch [3]: [PATCH 3/4] hwmon: (nzxt-smart2) Stop device IO before calling hid_hw_stop ---
commit f577839ac93bb28ac70b3214f4e4ddca1a015f5a
Author: Guenter Roeck <linux@roeck-us.net>

hwmon: (nzxt-smart2) Stop device IO before calling hid_hw_stop

Calling hid_hw_stop() does not stop the device IO, causing a race condition
between hid_input_report() and probe failure. Fix this by calling
hid_device_io_stop() before calling hid_hw_stop().

> diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c
> index 58ef9fa0184be..286de15cedc7d 100644
> --- a/drivers/hwmon/nzxt-smart2.c
> +++ b/drivers/hwmon/nzxt-smart2.c
> @@ -770,6 +770,7 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
>  	hid_hw_close(hdev);
>  
>  out_hw_stop:
> +	hid_device_io_stop(hdev);

[Severity: Low]
If hid_hw_open() fails earlier in the probe function, we jump to out_hw_stop
and call hid_device_io_stop():

drivers/hwmon/nzxt-smart2.c:nzxt_smart2_hid_probe()
        ret = hid_hw_open(hdev);
        if (ret)
                goto out_hw_stop;

Since hid_device_io_start() hasn't been called yet, does this trigger the
dev_warn("io already stopped") in hid_device_io_stop()?

>  	hid_hw_stop(hdev);
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but does ignoring the return value of
init_device() cause sysfs reads to hang?

If init_device() fails to send the 'detect fans' command during
nzxt_smart2_hid_probe(), it appears fan_config_received is never set to true.
Will subsequent sysfs reads block forever in
wait_event_interruptible_locked_irq() waiting for that condition?


[Severity: High]
This is a pre-existing issue, but does device removal hang if a user
process is currently blocked reading sysfs attributes?

When a process accesses sysfs attributes, nzxt_smart2_hwmon_read() blocks:

drivers/hwmon/nzxt-smart2.c:nzxt_smart2_hwmon_read()
        res = wait_event_interruptible_locked_irq(drvdata->wq,
                                                  drvdata->fan_config_received);

If the device is unplugged, nzxt_smart2_hid_remove() calls
hwmon_device_unregister(), which waits for active readers to finish:

drivers/hwmon/nzxt-smart2.c:nzxt_smart2_hid_remove()
        hwmon_device_unregister(drvdata->hwmon);

Since the waitqueue is never woken up during remove(), does the reader
process stay blocked indefinitely, causing the unregister to hang?


[Severity: Critical]
This isn't a bug introduced by this patch, but does the same use-after-free
vulnerability exist in the probe error paths of other HID hwmon drivers?

Both nzxt-kraken3.c and corsair-psu.c appear to call hid_device_io_start()
during probe, but omit hid_device_io_stop() in their fail_and_stop error
paths:

drivers/hwmon/nzxt-kraken3.c:kraken3_probe()
fail_and_close:
        hid_hw_close(hdev);
fail_and_stop:
        hid_hw_stop(hdev);

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

Do these drivers also need to call hid_device_io_stop(hdev) to prevent
hid_input_report() from executing after device structures are freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708015806.213790-1-linux@roeck-us.net?part=3