Re: [PATCH] hwmon: (corsair-psu) null terminate the vendor and product strings
Guenter Roeck <[email protected]> Sun, 2 Aug 2026 09:22:08 -0700
| Newsgroups | org.kernel.vger.linux-hwmon,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 8/2/26 06:42, Ali Ahmet Memis wrote:
> On Sun, Aug 02 2026, Wilken Gottwalt wrote:
>
>> The device always provides terminated strings. The vendor string ("CORSAIR"
>> or "Corsair") and the device string (3/4 numbers + 3 letters) are allways
>> around 10-16 bytes.
>
> Thanks, that settles it. So there is no reachable bug here, only a driver
> that relies on the device terminating the string. That changes what the
> patch should look like: v2 will say that outright, and I will drop the
> Fixes: tag, since this is hardening rather than a fix and it has no
> business going to stable.
>
>> Actually, it would make more sense to change the memcpy to
>> "REPLY_SIZE - 1". Just my thought.
>
> That works. One detail worth weighing before you pick: corsairpsu_usb_cmd()
> is also reached with a non NULL data from corsairpsu_request(), and
> corsairpsu_get_value() passes an uninitialized u8 data[REPLY_SIZE] on the
> stack. With REPLY_SIZE - 1 the last byte of that buffer is never written.
> Only data[0..3] are read, so nothing breaks, but the shortening is not
> confined to the two string callers.
>
> Either is fine by me and it is your driver, so say which you prefer and I
> will send v2 that way:
>
> a) memcpy(data, priv->cmd_buffer + 2, REPLY_SIZE - 1), arrays unchanged
> b) vendor and product as char[REPLY_SIZE + 1], memcpy unchanged
>
> No rush on the locking patch either, whenever you get to testing it.
>
Again, please stop fixing non-issues. We have enough real issues to deal with.
Thanks,
Guenter