Re: [PATCH] HID: validate report length and constants

[Davide Beatrici <me@davidebeatrici.dev>]

Thank you very much for your feedback!

I can send you a new identical device for testing if you would like.

> Can you show us what packets are emitted?
> 
> If it's a firmware bug, we better have a specific driver for it could
> be a HID-BPF program that just filters out the unwanted reports.
> 
> Also, how does Windows behave with this mouse? Does it need a special
> driver?

Sorry, I should've mentioned the malformed packet also shows up on 
Windows,
but is seemingly ignored because there appear to be no side effects 
whatsoever.

No special driver needed, it's detected as a standard HID mouse.

WireShark capture:
https://dl.houseof.software/misc/atk_x1_se_malformed_packet.pcapng

Packet screenshot:
https://dl.houseof.software/misc/atk_x1_se_malformed_packet.png

> Looks like there is something wrong either in the report descriptor of
> this mouse, either in the emitted reports.

Definitely. I have already informed the manufacturer, who confirmed the 
mouse
has only been tested on Windows.

My inquiry was forwarded to their R&D team, hopefully a firmware update 
will
be released soon.

> Yep, this is on purpose because Miscrosoft's own driver works that way
> and many HID devices do not bother to mark the non constant bits as
> data. So if you enforce the spec here, you'll break a numerous of
> devices unfortunatelly.

> Ouch. If I read you correctly, you are rejecting the entire report if a
> constant field is not 0. It is common for constant fields to be just
> garbage (whatever is in the memory, because writing firmware is hard),
> so even if we were to accept this patch, this would break even more
> devices :(

> I am pretty sure the HID selftests will fail with this patch applied,
> because there are probably a couple of devices there with the "non
> constant" behaviour.

Oh, in that case let's just drop that part from the patch, since it's
actually not altering the behavior with this specific device.

The malformed packet is detected and rejected by two checks:

Malformed report: raw_len=1 payload_len=1 expected=8 (ID 0)
Malformed report: const slice OOB (bit_off 8, len 8)