Re: [PATCH] hwmon: (corsair-psu) serialize debugfs access against hwmon

On Mon, 3 Aug 2026 16:19:57 -0700 Guenter Roeck wrote:
> Anyway, I (and Sashiko) think the patch is incomplete. It does not protect
> cmd_buffer when handling raw events (while executing corsairpsu_raw_event).
> That is a pre-existing issue, though. Not sure if that should be fixed in a
> separate patch or with this one. Thoughts ?

Separate, I think, and not really by choice: it cannot use the same lock.
corsairpsu_raw_event() is reached from the URB completion handler,
hid_irq_in() -> hid_safe_input_report() -> hdrv->raw_event(), and
Documentation/driver-api/usb/URB.rst is blunt about that path ("NEVER SLEEP
IN A COMPLETION HANDLER"). So whatever protects cmd_buffer there cannot be
the hwmon mutex this patch relies on, and has to be its own mechanism rather
than an extension of this one.

The window I see is the timeout. corsairpsu_usb_cmd() gives up with
-ETIMEDOUT, but the device can still deliver that reply afterwards. The next
command calls reinit_completion(), which makes completion_done() false again,
so the guard at the top of raw_event no longer rejects the late reply: it
memcpys into cmd_buffer and completes the new waiter with the previous
command's data. The echo check only catches that when the two commands
differ. For two reads of the same command on different rails it does not,
which is the same wrong-rail symptom this patch is about, reached the other
way round.

The two also blame differently, for whatever that is worth: this patch is
4207069edbf0, while the raw_event side goes back to d115b51e0e56 ("hwmon:
add Corsair PSU HID controller driver").

Nobody has written the raw_event one as far as I know, so tell me which way
you want it: I can send it, or leave it to you. Same for this patch, if you
would rather use the version you already had sitting around.