On Tue, Aug 24, 2021 at 1:54 PM Jiri Kosina [off-list ref] wrote:

On Fri, 20 Aug 2021, Alan Stern wrote:

quoted

quoted

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

That's good to know.  Still, I suspect there's a better way of handling
this condition.

In particular, does it make sense to accept descriptors for input or
feature reports with length zero?  I can't imagine what good such
reports would do.

I quickly went through drivers + some hidraw users, and can't spot any use
case for it.

quoted

On the other hand, I'm not familiar enough with the code to know the
right way to reject these descriptors and reports.  It looks like the
HID subsystem was not designed with this sort of check in mind.

Benjamin and Jiri, what do you think?  Is it okay to allow descriptors
for zero-length reports and just pretend they have length 1 (as the
patch tested by syzbot did), or should we instead reject them during
probing?

I think it's a good band-aid for 5.14 (or 5.14-stable if we don't make
it), and if it turns out to break something (which I don't expect), than
we can look into rejecting already during probe.

Benjamin, is there a way to run this quickly through your HID regression
testing machinery?

I have finally been able to test this patch:
- the testsuite is still passing (of course, this is not hid-core related)
- Logitech unify receivers are fine (according to the automated tests)
- Gaming mice with hidraw calls works (with libratbag in userspace)
- Wacom Intuos Pro still works (so the usbhid calls to enable the
tablet mode are still OK)

->
Tested-by: Benjamin Tissoires <redacted>
Acked-by: Benjamin Tissoires <redacted>

Alan, would you mind resending the patch with the various tags with a
commit description? (unless I missed it...)

Cheers,
Benjamin