Potential data race when spurious/unexpected reports are received

[jonasmalacofilho]

Hi!

I'm one the of the maintainers of liquidctl, and am investigating an issue that was reported to us (liquidctl/liquidctl#698). The original report involved two simultaneously processes (liquidctl and coolercontrol, the latter using liquidctl under the hood), both attempting to read the hwmon attributes exposed by this driver.

From time to time reading one of the attributes would fail with EOPNOTSUPP. As far as we know, no process was simultaneously accessing the device through hidraw (in that initial setup).

After checking the other, more usual, suspects, I think I may have found an issue with this kernel driver:

• (a) the driver uses a single buffer for both input and output;
• (b) ccp_raw_event overwrites the buffer if there's a completion pending;
• (c) send_usb_cmd reinits the completion (and therefore unlocks ccp_raw_event) before submitting the report.

With just (b) alone this means that any unexpected/spurious report from the device will, during a brief window, take the place of the actual report the driver is waiting for.

But because of (a) and (c), it's also possible for a spurious input report to change what command will be sent to the device next, possibly explaining why the device seems to report some invalid commands. And this would happen as part of a data race with hidcore/usbhid (so it might not just replace the command, but instead change it in rather hard to predict ways).

CC: @RayJW