Linux Hardware Monitor development
 help / color / mirror / Atom feed
* [PATCH] hwmon: (nzxt-smart2) Check return value of init_device() in probe
@ 2026-08-04  7:48 Qingshuang Fu
  2026-08-04  8:00 ` sashiko-bot
  2026-08-04 21:08 ` Guenter Roeck
  0 siblings, 2 replies; 4+ messages in thread
From: Qingshuang Fu @ 2026-08-04  7:48 UTC (permalink / raw)
  To: Guenter Roeck, Aleksandr Mezin
  Cc: linux-hwmon, linux-kernel, Qingshuang Fu, Qingshuang Fu

From: Qingshuang Fu <fuqingshuang@kylinos.cn>

The init_device() call in nzxt_smart2_hid_probe() can fail because it
sends HID output reports to the hardware to detect fans and set the
update interval.  If the hardware is not responding or the HID reports
fail, init_device() returns a negative error code.

However, the return value was ignored, causing the probe to continue
and register an hwmon device even though the device was never properly
initialized.  This leads to an inconsistent state where the driver
reports stale data or blocks on wait queues that will never be woken.

The same function's return value is already checked in the
reset_resume() handler, confirming the author's intent that errors
should be propagated.

Note that this fix was not possible before commit 59d104b54b0b
("hwmon: (nzxt-smart2) Stop device IO before calling hid_hw_stop")
because the out_hw_close error path was missing hid_device_io_stop(),
which would have opened a use-after-free risk window.

Fixes: 53e68c20aeb1 ("hwmon: add driver for NZXT RGB&Fan Controller/Smart Device v2.")
Signed-off-by: Qingshuang Fu <fuqingshuang@kylinos.cn>
---
 drivers/hwmon/nzxt-smart2.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c
index ff0c0bee0e83..efc78a8f58c8 100644
--- a/drivers/hwmon/nzxt-smart2.c
+++ b/drivers/hwmon/nzxt-smart2.c
@@ -754,7 +754,11 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
 
 	hid_device_io_start(hdev);
 
-	init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
+	ret = init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
+	if (ret) {
+		dev_err(&hdev->dev, "init_device failed: %d\n", ret);
+		goto out_hw_close;
+	}
 
 	drvdata->hwmon =
 		hwmon_device_register_with_info(&hdev->dev, "nzxtsmart2", drvdata,

base-commit: 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] hwmon: (nzxt-smart2) Check return value of init_device() in probe
  2026-08-04  7:48 [PATCH] hwmon: (nzxt-smart2) Check return value of init_device() in probe Qingshuang Fu
@ 2026-08-04  8:00 ` sashiko-bot
  2026-08-04  8:21   ` Qingshuang Fu
  2026-08-04 21:08 ` Guenter Roeck
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-08-04  8:00 UTC (permalink / raw)
  To: Qingshuang Fu; +Cc: linux-hwmon

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

Pre-existing issues:
- [High] Sysfs read and write handlers wait indefinitely for hardware reports without providing a mechanism to break the wait on device disconnect, causing the device unbind thread to deadlock.
--

--- Patch [1]: [PATCH] hwmon: (nzxt-smart2) Check return value of init_device() in probe ---
commit 18435050bb0408be0aa1e570c7a2976a468d52c0
Author: Qingshuang Fu <fuqingshuang@kylinos.cn>

hwmon: (nzxt-smart2) Check return value of init_device() in probe

This commit updates the nzxt-smart2 driver to check the return value of
init_device() during probe. If the hardware is unresponsive and initialization
fails, the driver now correctly aborts rather than registering the hwmon
device in an inconsistent state. This safely utilizes an earlier fix that
stops device IO before closing the hardware.

> diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c
> index ff0c0bee0e839..efc78a8f58c88 100644
> --- a/drivers/hwmon/nzxt-smart2.c
> +++ b/drivers/hwmon/nzxt-smart2.c
> @@ -754,7 +754,11 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
>  
>  	hid_device_io_start(hdev);
>  
> -	init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
> +	ret = init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
> +	if (ret) {
> +		dev_err(&hdev->dev, "init_device failed: %d\n", ret);
> +		goto out_hw_close;
> +	}
>  
>  	drvdata->hwmon =
>  		hwmon_device_register_with_info(&hdev->dev, "nzxtsmart2", drvdata,

[Severity: High]
This is a pre-existing issue, but does the driver handle device disconnects
safely while blocked in sysfs reads?

If a userspace program is reading a sysfs attribute (like pwm1_enable) and
blocks in nzxt_smart2_hwmon_read() because the device hasn't sent the
required report:

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 suddenly disconnected, nzxt_smart2_hid_remove() calls
hwmon_device_unregister() without waking up the wait queue or signaling a
disconnect. Will this cause a deadlock in kernfs_drain since the sysfs
thread never wakes up to finish its read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804074842.505923-1-fffsqian@163.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re:Re: [PATCH] hwmon: (nzxt-smart2) Check return value of init_device() in probe
  2026-08-04  8:00 ` sashiko-bot
@ 2026-08-04  8:21   ` Qingshuang Fu
  0 siblings, 0 replies; 4+ messages in thread
From: Qingshuang Fu @ 2026-08-04  8:21 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-hwmon


At 2026-08-04 16:00:01, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
>Pre-existing issues:
>- [High] Sysfs read and write handlers wait indefinitely for hardware reports without providing a mechanism to break the wait on device disconnect, causing the device unbind thread to deadlock.
>--
>
>--- Patch [1]: [PATCH] hwmon: (nzxt-smart2) Check return value of init_device() in probe ---
>commit 18435050bb0408be0aa1e570c7a2976a468d52c0
>Author: Qingshuang Fu <fuqingshuang@kylinos.cn>
>
>hwmon: (nzxt-smart2) Check return value of init_device() in probe
>
>This commit updates the nzxt-smart2 driver to check the return value of
>init_device() during probe. If the hardware is unresponsive and initialization
>fails, the driver now correctly aborts rather than registering the hwmon
>device in an inconsistent state. This safely utilizes an earlier fix that
>stops device IO before closing the hardware.
>
>> diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c
>> index ff0c0bee0e839..efc78a8f58c88 100644
>> --- a/drivers/hwmon/nzxt-smart2.c
>> +++ b/drivers/hwmon/nzxt-smart2.c
>> @@ -754,7 +754,11 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
>>  
>>  	hid_device_io_start(hdev);
>>  
>> -	init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
>> +	ret = init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
>> +	if (ret) {
>> +		dev_err(&hdev->dev, "init_device failed: %d\n", ret);
>> +		goto out_hw_close;
>> +	}
>>  
>>  	drvdata->hwmon =
>>  		hwmon_device_register_with_info(&hdev->dev, "nzxtsmart2", drvdata,
>
>[Severity: High]
>This is a pre-existing issue, but does the driver handle device disconnects
>safely while blocked in sysfs reads?
>
>If a userspace program is reading a sysfs attribute (like pwm1_enable) and
>blocks in nzxt_smart2_hwmon_read() because the device hasn't sent the
>required report:
>
>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 suddenly disconnected, nzxt_smart2_hid_remove() calls
>hwmon_device_unregister() without waking up the wait queue or signaling a
>disconnect. Will this cause a deadlock in kernfs_drain since the sysfs
>thread never wakes up to finish its read?
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20260804074842.505923-1-fffsqian@163.com?part=1

Thanks for the automated review.

The deadlock scenario you noted is a pre-existing issue within the driver and falls outside the scope of this patch.
This change addresses failures occurring during early probe. If init_device() returns an error, the probe aborts before hwmon_device_register_with_info() is called. Since no sysfs attributes are created in this error path, there is no opportunity for userspace readers to block on wait_event.

I would like to keep this patch narrowly focused on preventing registration of an incompletely initialized hwmon device, and not expand it to cover other unrelated existing problems.

Qingshuang Fu

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] hwmon: (nzxt-smart2) Check return value of init_device() in probe
  2026-08-04  7:48 [PATCH] hwmon: (nzxt-smart2) Check return value of init_device() in probe Qingshuang Fu
  2026-08-04  8:00 ` sashiko-bot
@ 2026-08-04 21:08 ` Guenter Roeck
  1 sibling, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2026-08-04 21:08 UTC (permalink / raw)
  To: Qingshuang Fu; +Cc: Aleksandr Mezin, linux-hwmon, linux-kernel, Qingshuang Fu

On Tue, Aug 04, 2026 at 03:48:42PM +0800, Qingshuang Fu wrote:
> From: Qingshuang Fu <fuqingshuang@kylinos.cn>
> 
> The init_device() call in nzxt_smart2_hid_probe() can fail because it
> sends HID output reports to the hardware to detect fans and set the
> update interval.  If the hardware is not responding or the HID reports
> fail, init_device() returns a negative error code.
> 
> However, the return value was ignored, causing the probe to continue
> and register an hwmon device even though the device was never properly
> initialized.  This leads to an inconsistent state where the driver
> reports stale data or blocks on wait queues that will never be woken.
> 
> The same function's return value is already checked in the
> reset_resume() handler, confirming the author's intent that errors
> should be propagated.
> 
> Note that this fix was not possible before commit 59d104b54b0b
> ("hwmon: (nzxt-smart2) Stop device IO before calling hid_hw_stop")
> because the out_hw_close error path was missing hid_device_io_stop(),
> which would have opened a use-after-free risk window.
> 
> Fixes: 53e68c20aeb1 ("hwmon: add driver for NZXT RGB&Fan Controller/Smart Device v2.")
> Signed-off-by: Qingshuang Fu <fuqingshuang@kylinos.cn>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/nzxt-smart2.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> 
> base-commit: 848acc8ffe1b7cd5f1bf427b93069becfebc2c9d
> 
> diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c
> index ff0c0bee0e83..efc78a8f58c8 100644
> --- a/drivers/hwmon/nzxt-smart2.c
> +++ b/drivers/hwmon/nzxt-smart2.c
> @@ -754,7 +754,11 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
>  
>  	hid_device_io_start(hdev);
>  
> -	init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
> +	ret = init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
> +	if (ret) {
> +		dev_err(&hdev->dev, "init_device failed: %d\n", ret);
> +		goto out_hw_close;
> +	}
>  
>  	drvdata->hwmon =
>  		hwmon_device_register_with_info(&hdev->dev, "nzxtsmart2", drvdata,

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-04 21:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04  7:48 [PATCH] hwmon: (nzxt-smart2) Check return value of init_device() in probe Qingshuang Fu
2026-08-04  8:00 ` sashiko-bot
2026-08-04  8:21   ` Qingshuang Fu
2026-08-04 21:08 ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox