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