I've looked through corsair-psu sources, and I think filtering in raw_event won't be enough. For example, in corsairpsu_request, there are 2 commands, and they should be executed consecutively: 1) corsairpsu_usb_cmd(priv, 2, PSU_CMD_SELECT_RAIL, rail, NULL); 2) corsairpsu_usb_cmd(priv, 3, cmd, 0, data); If the userspace will squeeze another PSU_CMD_SELECT_RAIL between (1) and (2), the driver will get data for a wrong rail (and with the current code won't even notice it). So unless there is a way to "lock" hidraw (and it seems that there isn't - looking at the code, hidraw calls the low-level hid driver directly, as far as I understand), it won't work correctly. And if a driver can't work correctly with hidraw enabled - maybe it shouldn't enable hidraw? So that the user can 1) notice that something is wrong 2) blacklist or unbind the driver (or userspace tools that use hidraw can unbind automatically). Otherwise everything seems to be silently broken. On the other hand, maybe races between the kernel driver and userspace tools are unlikely, because the driver doesn't talk to the device continuously - only when sysfs reads happen. Added corsair-psu maintainer to Cc: On Thu, Jun 17, 2021 at 7:14 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > On Thu, Jun 17, 2021 at 01:11:38PM +0600, Aleksandr Mezin wrote: > > On Thu, Jun 17, 2021 at 12:27 PM Marius Zachmann <mail@xxxxxxxxxxxxxxxxx> wrote: > > ... > > > I do not know, what your device is doing > > > > Actually, NZXT devices (at least grid/"smart device" and "smart device > > v2"/rgb&fan controller) don't have such issues - they use report ids, > > and don't even expect request-reply communication pattern. I've just > > noticed that something seems to be wrong with corsair-cpro (but > > somehow didn't notice the comment) and decided to ask. > > > > > This device uses an echo of the command > > > in the answer and if they don't match it returns an error. This could > > > maybe lead to a false error when the replies are switched, but is > > > probably preferable. > > > > Hm... If the response includes the id of the request, it should be > > possible to filter reports in raw_event, i. e. don't signal completion > > if the report doesn't match, and wait more. Yes, there is a corner > > case, "if a command is not supported, the length value in the reply is > > okay, but the command value is set to 0". But timing out (250 ms) in > > this case should probably be fine... Actually I have a compatible > > Corsair PSU so maybe I'll send a patch. > > Patches to improve the situation are welcome. My understanding is > that with the current driver users should disable the kernel driver > if they plan to use userspace tools to access the device. > > Guenter
