[PATCH v2 0/4] Fix error handling in HID device probe functions

[PATCH v2 0/4] Fix error handling in HID device probe functions

[Guenter Roeck]

Calling hid_hw_stop() does not stop the device IO.
This results in a race condition between hid_input_report() and the point
immediately following the execution of hid_device_io_start() within
driver probe functions. If the probe operation fails after "io start"
has been initiated, this race condition will result in a UAF vulnerability.

Fix the problem by calling hid_device_io_stop() before calling
hid_hw_stop() in affected hardware monitoring drivers.

v2: Call hid_device_io_stop() only if hid_device_io_start() has been called

----------------------------------------------------------------
Guenter Roeck (4):
      hwmon: (corsair-cpro) Stop device IO before calling hid_hw_stop
      hwmon: (gigabyte_waterforce) Stop device IO before calling hid_hw_stop
      hwmon: (nzxt-smart2) Stop device IO before calling hid_hw_stop
      hwmon: (nzxt-kraken3) Stop device IO before calling hid_hw_stop

 drivers/hwmon/corsair-cpro.c        | 1 +
 drivers/hwmon/gigabyte_waterforce.c | 4 +++-
 drivers/hwmon/nzxt-kraken3.c        | 6 ++++--
 drivers/hwmon/nzxt-smart2.c         | 2 +-
 4 files changed, 9 insertions(+), 4 deletions(-)

[PATCH v2 1/4] hwmon: (corsair-cpro) Stop device IO before calling hid_hw_stop

[Guenter Roeck]

Calling hid_hw_stop() does not stop the device IO.
This results in a race condition between hid_input_report() and the point
immediately following the execution of hid_device_io_start() within
the driver probe function. If the probe operation fails after "io start"
has been initiated, this race condition will result in a UAF vulnerability.

Fix the problem by calling hid_device_io_stop() before calling
hid_hw_stop().

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 40c3a44542257 ("hwmon: add Corsair Commander Pro driver")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Call hid_device_io_stop() only if hid_device_io_start() has been called

 drivers/hwmon/corsair-cpro.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c
index b6e508e43fa1..8354a002f4c5 100644
--- a/drivers/hwmon/corsair-cpro.c
+++ b/drivers/hwmon/corsair-cpro.c
@@ -645,6 +645,7 @@ static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 out_hw_close:
 	hid_hw_close(hdev);
+	hid_device_io_stop(hdev);
 out_hw_stop:
 	hid_hw_stop(hdev);
 	return ret;
-- 
2.45.2

Re: [PATCH v2 1/4] hwmon: (corsair-cpro) Stop device IO before calling hid_hw_stop

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing cleanup of debugfs files in probe error path leads to a Use-After-Free vulnerability.
--

--- Patch [1]: [PATCH v2 1/4] hwmon: (corsair-cpro) Stop device IO before calling hid_hw_stop ---
commit 93ea292134a6e9bcca068675cf1869020bdac096
Author: Guenter Roeck <linux@roeck-us.net>

hwmon: (corsair-cpro) Stop device IO before calling hid_hw_stop

This commit fixes a UAF vulnerability caused by a race condition between
hid_input_report() and driver probe failure by calling hid_device_io_stop()
before hid_hw_stop().

> diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c
> index b6e508e43fa17..8354a002f4c5e 100644
> --- a/drivers/hwmon/corsair-cpro.c
> +++ b/drivers/hwmon/corsair-cpro.c
> @@ -645,6 +645,7 @@ static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  
>  out_hw_close:
>  	hid_hw_close(hdev);
> +	hid_device_io_stop(hdev);

[Severity: High]
This is a pre-existing issue, but does this error path also need to clean up
the debugfs files?

In ccp_probe(), the debugfs files are created just before registering the
hwmon device:

	ccp_debugfs_init(ccp);

	ccp->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, ...
	if (IS_ERR(ccp->hwmon_dev)) {
		ret = PTR_ERR(ccp->hwmon_dev);
		goto out_hw_close;
	}

If hwmon_device_register_with_info() fails, we jump here to out_hw_close, but
we don't remove the newly created ccp->debugfs. 

Since devres automatically frees the ccp structure when the probe function
returns, this leaves dangling files in debugfs.

If a user-space process later reads these leaked files, won't callback
functions like firmware_show() dereference the freed ccp pointer and cause
a Use-After-Free?

>  out_hw_stop:
>  	hid_hw_stop(hdev);
>  	return ret;

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

[PATCH v2 2/4] hwmon: (gigabyte_waterforce) Stop device IO before calling hid_hw_stop

[Guenter Roeck]

Calling hid_hw_stop() does not stop the device IO.
This results in a race condition between hid_input_report() and the point
immediately following the execution of hid_device_io_start() within
the driver probe function. If the probe operation fails after "io start"
has been initiated, this race condition will result in a UAF vulnerability.

Fix the problem by calling hid_device_io_stop() before calling
hid_hw_stop().

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 42ac68e3d4ba0 ("hwmon: Add driver for Gigabyte AORUS Waterforce AIO coolers")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Call hid_device_io_stop() only if hid_device_io_start() has been called

 drivers/hwmon/gigabyte_waterforce.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/gigabyte_waterforce.c b/drivers/hwmon/gigabyte_waterforce.c
index 27487e215bdd..4eea05f8b569 100644
--- a/drivers/hwmon/gigabyte_waterforce.c
+++ b/drivers/hwmon/gigabyte_waterforce.c
@@ -371,13 +371,15 @@ static int waterforce_probe(struct hid_device *hdev, const struct hid_device_id
 	if (IS_ERR(priv->hwmon_dev)) {
 		ret = PTR_ERR(priv->hwmon_dev);
 		hid_err(hdev, "hwmon registration failed with %d\n", ret);
-		goto fail_and_close;
+		goto fail_and_io_stop;
 	}
 
 	waterforce_debugfs_init(priv);
 
 	return 0;
 
+fail_and_io_stop:
+	hid_device_io_stop(hdev);
 fail_and_close:
 	hid_hw_close(hdev);
 fail_and_stop:
-- 
2.45.2

Re: [PATCH v2 2/4] hwmon: (gigabyte_waterforce) Stop device IO before calling hid_hw_stop

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Systemic Preexisting UAF in other hwmon HID drivers.
- [High] Architecture Violation and Data Race in hwmon sensor reading.
- [High] Potential Deadlock between process and hardirq context.
--

--- Patch [2]: [PATCH v2 2/4] hwmon: (gigabyte_waterforce) Stop device IO before calling hid_hw_stop ---
commit 1ac1936efdffad20d37444790d9aa7b430d348cd
Author: Guenter Roeck <linux@roeck-us.net>

hwmon: (gigabyte_waterforce) Stop device IO before calling hid_hw_stop

Calling hid_hw_stop() does not stop the device IO. This results in a race
condition between hid_input_report() and the point immediately following the
execution of hid_device_io_start() within the driver probe function. Fix the
problem by calling hid_device_io_stop() before calling hid_hw_stop().

> diff --git a/drivers/hwmon/gigabyte_waterforce.c b/drivers/hwmon/gigabyte_waterforce.c
> index 27487e215bddf..4eea05f8b569c 100644
> --- a/drivers/hwmon/gigabyte_waterforce.c
> +++ b/drivers/hwmon/gigabyte_waterforce.c
> @@ -371,13 +371,15 @@ static int waterforce_probe(struct hid_device *hdev, const struct hid_device_id
>  	if (IS_ERR(priv->hwmon_dev)) {
>  		ret = PTR_ERR(priv->hwmon_dev);
>  		hid_err(hdev, "hwmon registration failed with %d\n", ret);
> -		goto fail_and_close;
> +		goto fail_and_io_stop;
>  	}
>  
>  	waterforce_debugfs_init(priv);
>  
>  	return 0;
>  
> +fail_and_io_stop:
> +	hid_device_io_stop(hdev);
>  fail_and_close:
>  	hid_hw_close(hdev);
>  fail_and_stop:

[Severity: Critical]
This isn't a bug introduced by this patch, but does this same UAF
vulnerability exist in other hwmon HID drivers?

Looking at corsairpsu_probe() in drivers/hwmon/corsair-psu.c, kraken3_probe()
in drivers/hwmon/nzxt-kraken3.c, and nzxt_smart2_hid_probe() in
drivers/hwmon/nzxt-smart2.c, the error paths jump directly to hid_hw_close()
and hid_hw_stop() without calling hid_device_io_stop(). For example, in
corsairpsu_probe:

fail_and_close:
	hid_hw_close(hdev);
fail_and_stop:
	hid_hw_stop(hdev);
	return ret;

Since hid_device_io_start() unlocked the driver input lock, asynchronous HID
events could continue firing and triggering the raw_event handler while the
driver state is freed. Should these other drivers be fixed as well?

[Severity: High]
This is a pre-existing issue, but does the lack of locking when reading sensor
state cause torn reads?

In drivers/hwmon/gigabyte_waterforce.c, waterforce_read() reads the
priv->temp_input[channel] array directly from process context:

    *val = priv->temp_input[channel];

Concurrently, waterforce_raw_event() runs in the HID input/interrupt context
and modifies these identical arrays:

    priv->temp_input[0] = data[WATERFORCE_TEMP_SENSOR] * 1000;
    priv->speed_input[0] = get_unaligned_le16(data + WATERFORCE_FAN_SPEED);

According to the subsystem guidelines in subsystem/hwmon.md, drivers must
implement locking required for interrupt handling (e.g., using hwmon_lock()
and hwmon_unlock()). Could the lack of synchronization here allow the
interrupt handler to tear the data being read by sysfs?

[Severity: High]
This isn't a bug introduced by this patch, but could the lock usage around
priv->status_report_request_lock lead to a deadlock?

In drivers/hwmon/gigabyte_waterforce.c, waterforce_get_status() acquires the
lock using spin_lock_bh(), which disables softirqs but leaves hardirqs
enabled:

    spin_lock_bh(&priv->status_report_request_lock);
    reinit_completion(&priv->status_report_received);
    spin_unlock_bh(&priv->status_report_request_lock);

If a USB URB completion fires in hardirq context while this lock is held, it
will call waterforce_raw_event(), which attempts to acquire the same lock
using plain spin_lock():

    spin_lock(&priv->status_report_request_lock);
    if (!completion_done(&priv->status_report_received))
        complete_all(&priv->status_report_received);
    spin_unlock(&priv->status_report_request_lock);

Could this cause a recursive spinlock deadlock? Would it be safer to use
spin_lock_irqsave() and spin_lock_irqrestore() in both paths to properly
synchronize between process context and the HID raw event handler?

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

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

[Guenter Roeck]

Calling hid_hw_stop() does not stop the device IO.
This results in a race condition between hid_input_report() and the point
immediately following the execution of hid_device_io_start() within
the driver probe function. If the probe operation fails after "io start"
has been initiated, this race condition will result in a UAF vulnerability.

Fix the problem by calling hid_device_io_stop() before calling
hid_hw_stop().

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 53e68c20aeb1e ("hwmon: add driver for NZXT RGB&Fan Controller/Smart Device v2")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Call hid_device_io_stop() only if hid_device_io_start() has been called

 drivers/hwmon/nzxt-smart2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c
index 58ef9fa0184b..e2316c46629d 100644
--- a/drivers/hwmon/nzxt-smart2.c
+++ b/drivers/hwmon/nzxt-smart2.c
@@ -768,7 +768,7 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
 
 out_hw_close:
 	hid_hw_close(hdev);
-
+	hid_device_io_stop(hdev);
 out_hw_stop:
 	hid_hw_stop(hdev);
 	return ret;
-- 
2.45.2

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] The patch fixes a UAF vulnerability in `nzxt-smart2.c` by calling `hid_device_io_stop()` on probe failure after `hid_device_io_start()`, but fails to apply this same necessary fix to multiple other HID drivers that suffer from the exact same vulnerability.
- [Critical] The output buffer embedded in `struct drvdata` is not cacheline aligned, violating DMA API rules.
- [High] Device unplug causes a deadlock in hwmon_device_unregister() due to sysfs readers stuck in indefinite wait.
- [High] The return value of `init_device()` in `nzxt_smart2_hid_probe()` is completely ignored, which can lead to indefinite hangs during sysfs reads.
- [High] Potential deadlock between process context and hardware interrupt context due to incorrect spinlock variants.
--

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

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

This commit addresses a race condition between hid_input_report and the
probe function. It calls hid_device_io_stop before hid_hw_stop in the error
path to prevent a use-after-free vulnerability.

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

[Severity: Critical]
This isn't a bug introduced by this patch, but while addressing the
use-after-free vulnerability here, are there other drivers that need this
same fix? 

For example, in drivers/hwmon/corsair-psu.c:corsairpsu_probe():

    fail_and_close:
        hid_hw_close(hdev);
    fail_and_stop:
        hid_hw_stop(hdev);

And similarly in drivers/hwmon/nzxt-kraken3.c:kraken3_probe(), 
drivers/hid/hid-nintendo.c:nintendo_hid_probe(), and 
drivers/hid/hid-mcp2221.c:mcp2221_hid_unregister(). They also appear to omit 
hid_device_io_stop() in their cleanup paths after hid_device_io_start() has 
been called. Could they be vulnerable to the exact same issue?

>  out_hw_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 indefinitely?

In nzxt_smart2_hid_probe():

    hid_device_io_start(hdev);
    init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
    drvdata->hwmon = hwmon_device_register_with_info(...)

If init_device() fails, the device won't send the expected FAN_CONFIG input 
report. When a user process later attempts to read from sysfs, it triggers 
nzxt_smart2_hwmon_read():

    res = wait_event_interruptible_locked_irq(drvdata->wq,
                                              drvdata->fan_config_received);

Will this wait loop block forever waiting for a configuration report that
will never arrive?

[Severity: High]
This isn't a bug introduced by this patch, but can the driver deadlock if 
the device is physically unplugged while a sysfs read is pending?

If a sysfs read is waiting in nzxt_smart2_hwmon_read() as shown above, and 
the device is disconnected, nzxt_smart2_hid_remove() is called:

    static void nzxt_smart2_hid_remove(struct hid_device *hdev)
    {
        struct drvdata *drvdata = hid_get_drvdata(hdev);
        hwmon_device_unregister(drvdata->hwmon);

Because the sysfs read is blocked indefinitely waiting for an input report 
from the unplugged device, will hwmon_device_unregister() hang in 
kernfs_drain() waiting for the active sysfs operation to finish?

[Severity: High]
This is a pre-existing issue, but is there a potential deadlock between 
process context and hardware interrupt context here?

In set_pwm(), the lock is acquired with softirqs disabled but hardirqs 
enabled:

    spin_lock_bh(&drvdata->wq.lock);
    drvdata->fan_duty_percent[channel] = duty_percent;
    spin_unlock_bh(&drvdata->wq.lock);

If a URB completion hardware interrupt fires on the same CPU while this 
lock is held, it can call handle_fan_status_report():

    spin_lock(&drvdata->wq.lock);

Will the CPU spin forever attempting to acquire the lock that the 
interrupted process already holds?

[Severity: Critical]
This isn't a bug introduced by this patch, but does passing an embedded 
buffer to the HID core violate DMA API rules?

In struct drvdata:

    struct mutex mutex;
    long update_interval;
    u8 output_buffer[OUTPUT_REPORT_SIZE];

In send_output_report():

    ret = hid_hw_output_report(drvdata->hid, drvdata->output_buffer,
                               sizeof(drvdata->output_buffer));

Since output_buffer shares a cacheline with the mutex and update_interval,
could the underlying USB DMA mapping operations corrupt these adjacent
variables on systems with non-coherent caches?

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

[PATCH v2 4/4] hwmon: (nzxt-kraken3) Stop device IO before calling hid_hw_stop

[Guenter Roeck]

Calling hid_hw_stop() does not stop the device IO.
This results in a race condition between hid_input_report() and the point
immediately following the execution of hid_device_io_start() within
the driver probe function. If the probe operation fails after "io start"
has been initiated, this race condition will result in a UAF vulnerability.

Fix the problem by calling hid_device_io_stop() before calling
hid_hw_stop().

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: f3b4b146eb107 ("hwmon: Add driver for NZXT Kraken X and Z series AIO CPU coolers")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Call hid_device_io_stop() only if hid_device_io_start() has been called

 drivers/hwmon/nzxt-kraken3.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/nzxt-kraken3.c b/drivers/hwmon/nzxt-kraken3.c
index d00409bcab93..05525406c5fb 100644
--- a/drivers/hwmon/nzxt-kraken3.c
+++ b/drivers/hwmon/nzxt-kraken3.c
@@ -948,7 +948,7 @@ static int kraken3_probe(struct hid_device *hdev, const struct hid_device_id *id
 	ret = kraken3_init_device(hdev);
 	if (ret < 0) {
 		hid_err(hdev, "device init failed with %d\n", ret);
-		goto fail_and_close;
+		goto fail_and_stop_io;
 	}
 
 	ret = kraken3_get_fw_ver(hdev);
@@ -960,13 +960,15 @@ static int kraken3_probe(struct hid_device *hdev, const struct hid_device_id *id
 	if (IS_ERR(priv->hwmon_dev)) {
 		ret = PTR_ERR(priv->hwmon_dev);
 		hid_err(hdev, "hwmon registration failed with %d\n", ret);
-		goto fail_and_close;
+		goto fail_and_stop_io;
 	}
 
 	kraken3_debugfs_init(priv, device_name);
 
 	return 0;
 
+fail_and_stop_io:
+	hid_device_io_stop(hdev);
 fail_and_close:
 	hid_hw_close(hdev);
 fail_and_stop:
-- 
2.45.2

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing hwmon_lock() serialization for sysfs extra_groups attributes in kraken3_fan_curve_pwm_store.
- [High] Insufficient bounds checking in kraken3_raw_event leads to an out-of-bounds read for Z53 devices.
- [High] Probe error paths in multiple other HID drivers fail to call `hid_device_io_stop(hdev)`, leaving IO enabled and leading to UAF vulnerabilities.
- [Medium] Using `spin_lock_bh()` in process context while the same lock is acquired with `spin_lock()` in a hardirq-capable completion handler leads to deadlocks.
- [Medium] Sensor readings, update timestamps, and firmware version arrays are modified by the interrupt handler locklessly and read by sysfs/debugfs callbacks locklessly.
--

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

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

Calling hid_hw_stop() does not stop the device IO, which can result
in a use-after-free race condition between hid_input_report() and probe
failure. This fix adds hid_device_io_stop() before hid_hw_stop() to
prevent the vulnerability.

> diff --git a/drivers/hwmon/nzxt-kraken3.c b/drivers/hwmon/nzxt-kraken3.c
> index d00409bcab93a..05525406c5fbb 100644
> --- a/drivers/hwmon/nzxt-kraken3.c
> +++ b/drivers/hwmon/nzxt-kraken3.c
> @@ -960,13 +960,15 @@ static int kraken3_probe(struct hid_device *hdev, const struct hid_device_id *id
[ ... ]
>  	return 0;
>  
> +fail_and_stop_io:
> +	hid_device_io_stop(hdev);
>  fail_and_close:
>  	hid_hw_close(hdev);
>  fail_and_stop:
>  	hid_hw_stop(hdev);
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but do other HID drivers share this same
missing cleanup?

Looking at corsairpsu_probe() in drivers/hwmon/corsair-psu.c:

fail_and_close:
	hid_hw_close(hdev);
fail_and_stop:
	hid_hw_stop(hdev);
	return ret;

And mcp2221_probe() in drivers/hid/hid-mcp2221.c:

	ret = mcp_set_i2c_speed(mcp);
	if (ret) {
		hid_err(hdev, "can't set i2c speed: %d\n", ret);
		return ret;
	}

And nintendo_hid_probe() in drivers/hid/hid-nintendo.c:

err_close:
	hid_hw_close(hdev);
err_stop:
	hid_hw_stop(hdev);

Could these error paths also leave IO enabled and trigger a similar UAF
vulnerability if the HID core processes events after devres frees the
private data?

[Severity: High]
This isn't a bug introduced by this patch, but does the custom attribute
store function lack hwmon_lock() serialization?

drivers/hwmon/nzxt-kraken3.c:kraken3_fan_curve_pwm_store() {
...
		ret =
		    kraken3_write_curve(priv,
					priv->channel_info[dev_attr->nr].pwm_points, dev_attr->nr);
...
}

The hwmon core only serializes attributes registered via the info
parameter. For attributes registered via extra_groups, can modifying
the fan curve arrays without hwmon_lock() or priv->buffer_lock allow
concurrent sysfs writes to race with the core hwmon write callback and
corrupt device state?

[Severity: High]
This is a pre-existing issue, but does kraken3_raw_event() have
sufficient bounds checking for Z53 devices?

drivers/hwmon/nzxt-kraken3.c:kraken3_raw_event() {
...
		priv->fan_input[1] = get_unaligned_le16(data + Z53_FAN_SPEED_OFFSET);
		priv->channel_info[1].reported_duty =
		    kraken3_percent_to_pwm(data[Z53_FAN_DUTY_OFFSET]);
...
}

The function checks if the size is at least MIN_REPORT_LENGTH (20
bytes). If a report is exactly 20 bytes, does unconditionally accessing
offsets 23, 24, and 25 cause an out-of-bounds read?

[Severity: Medium]
This isn't a bug introduced by this patch, but is there a deadlock risk
between kraken3_read_z53() and kraken3_raw_event()?

kraken3_read_z53() uses spin_lock_bh():
	spin_lock_bh(&priv->status_completion_lock);

kraken3_raw_event() uses spin_lock():
	spin_lock(&priv->status_completion_lock);

On systems where legacy USB host controllers execute URB givebacks in
hardirq context, could the interrupt handler attempt to acquire the
already held spinlock and deadlock on the local CPU?

[Severity: Medium]
This is a pre-existing issue, but are sensor readings modified
locklessly?

drivers/hwmon/nzxt-kraken3.c:kraken3_raw_event() {
...
	/* Temperature and fan sensor readings */
	priv->temp_input[0] =
	    data[TEMP_SENSOR_START_OFFSET] * 1000 + data[TEMP_SENSOR_END_OFFSET] * 100;
...
}

And read in kraken3_read() without locks:

drivers/hwmon/nzxt-kraken3.c:kraken3_read() {
...
	case hwmon_temp_input:
		*val = priv->temp_input[channel];
...
}

Can the sysfs/debugfs callbacks read these variables concurrently with
the interrupt handler and return torn reads, contrary to the hwmon
subsystem guidelines?

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