[PATCH] hwmon: corsair-psu: fix possible out-of-bounds access on missing string termination

[PATCH] hwmon: corsair-psu: fix possible out-of-bounds access on missing string termination

[Wilken Gottwalt]

In theory it could be possible that the REPLY_SIZE sized buffers for
holding the vendor and product strings could be end up missing the null
termination (for example by malicious hardware built on purpose)
required by the seq_printf() call. That limits the debugfs printf calls
to a maximum string length of REPLY_SIZE.

Fixes: d115b51e0e567 ("hwmon: add Corsair PSU HID controller driver")
Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
---
 drivers/hwmon/corsair-psu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
index ce958cdaef58..3cb0ba592250 100644
--- a/drivers/hwmon/corsair-psu.c
+++ b/drivers/hwmon/corsair-psu.c
@@ -701,7 +701,7 @@ static int vendor_show(struct seq_file *seqf, void *unused)
 {
 	struct corsairpsu_data *priv = seqf->private;
 
-	seq_printf(seqf, "%s\n", priv->vendor);
+	seq_printf(seqf, "%.*s\n", REPLY_SIZE, priv->vendor);
 
 	return 0;
 }
@@ -711,7 +711,7 @@ static int product_show(struct seq_file *seqf, void *unused)
 {
 	struct corsairpsu_data *priv = seqf->private;
 
-	seq_printf(seqf, "%s\n", priv->product);
+	seq_printf(seqf, "%.*s\n", REPLY_SIZE, priv->product);
 
 	return 0;
 }
-- 
2.55.0

Re: [PATCH] hwmon: corsair-psu: fix possible out-of-bounds access on missing string termination

[sashiko-bot]

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

Pre-existing issues:
- [High] Data race on shared `priv->cmd_buffer` and `priv->wait_completion` due to missing driver-level locking for debugfs attributes.
--

--- Patch [1]: [PATCH] hwmon: corsair-psu: fix possible out-of-bounds access on missing string termination ---
commit cfc2e894e816ca1d7330d9074123e16e3ad443b7
Author: Wilken Gottwalt <wilken.gottwalt@posteo.net>

hwmon: corsair-psu: fix possible out-of-bounds access on missing string termination

This commit limits the debugfs seq_printf() calls for vendor and product
strings to a maximum length of REPLY_SIZE. This prevents potential
out-of-bounds accesses if the strings are not properly null-terminated by
the hardware.

> diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
> index ce958cdaef58a..3c01ae3fc4af2 100644
> --- a/drivers/hwmon/corsair-psu.c
> +++ b/drivers/hwmon/corsair-psu.c
> @@ -701,7 +701,7 @@ static int vendor_show(struct seq_file *seqf, void *unused)
>  {
>  	struct corsairpsu_data *priv = seqf->private;
>  
> -	seq_printf(seqf, "%s\n", priv->vendor);
> +	seq_printf(seqf, "%.*s\n", REPLY_SIZE, priv->vendor);
>  
>  	return 0;
>  }
> @@ -711,7 +711,7 @@ static int product_show(struct seq_file *seqf, void *unused)
>  {
>  	struct corsairpsu_data *priv = seqf->private;
>  
> -	seq_printf(seqf, "%s\n", priv->product);
> +	seq_printf(seqf, "%.*s\n", REPLY_SIZE, priv->product);
>  
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but does the lack of driver-level locking
cause a data race on priv->cmd_buffer and priv->wait_completion when debugfs
attributes are read concurrently with sysfs attributes?

Looking at print_uptime(), it calls into corsairpsu_get_value() bypassing
the hwmon core sysfs locks:

drivers/hwmon/corsair-psu.c:print_uptime() {
    ...
	ret = corsairpsu_get_value(priv, cmd, 0, &val);
    ...
}

This proceeds into corsairpsu_usb_cmd() where the shared buffer and
completion are mutated without any synchronization lock:

drivers/hwmon/corsair-psu.c:corsairpsu_usb_cmd() {
    ...
	memset(priv->cmd_buffer, 0, CMD_BUFFER_SIZE);
	priv->cmd_buffer[0] = p0;
	priv->cmd_buffer[1] = p1;
	priv->cmd_buffer[2] = p2;

	reinit_completion(&priv->wait_completion);
    ...
}

The hwmon subsystem requires drivers to implement internal locking for
attributes registered outside the core, such as debugfs interfaces. Without
a transaction lock, interleaved USB commands can corrupt hardware state and
result in mismatched telemetry data.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/anLj9gPWRoRDbQBV@monster.localdomain?part=1

Re: [PATCH] hwmon: corsair-psu: fix possible out-of-bounds access on missing string termination

[Guenter Roeck]

On Wed, Aug 05, 2026 at 07:19:20AM +0000, Wilken Gottwalt wrote:
> In theory it could be possible that the REPLY_SIZE sized buffers for
> holding the vendor and product strings could be end up missing the null
> termination (for example by malicious hardware built on purpose)
> required by the seq_printf() call. That limits the debugfs printf calls
> to a maximum string length of REPLY_SIZE.
> 
> Fixes: d115b51e0e567 ("hwmon: add Corsair PSU HID controller driver")
> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>

Applied.

Thanks,
Guenter