Re: [PATCH 2/2] HID: check for valid USB device for many HID drivers

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Wed, Dec 01, 2021 at 05:47:20PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Dec 01, 2021 at 05:38:49PM +0100, Benjamin Tissoires wrote:
> > 
> > 
> > On Wed, Dec 1, 2021 at 4:05 PM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> > > > > FWIW, I am also working on a test that generates all registered USB
> > > > > vid/pids from the currently running kernel and will try to inject them
> > > > > in the testsuite. This should validate this patch and prevent future
> > > > > mishaps.
> > > >
> > > > That would be great to see.  Let me know if you want these broken up
> > > > smaller, and I will go fix the above issues.
> > > 
> > > https://gitlab.freedesktop.org/bentiss/hid-tools/-/commit/2b04c6cf46c7405c8a916f324988660cb54dadea
> > > 
> > > This seems sufficient to make an unpatched kernel scream :)
> > 
> > I have now opened https://gitlab.freedesktop.org/libevdev/hid-tools/-/merge_requests/122
> > with a quicker solution.
> > 
> > This leads apparently to 2 extra crashes:
> > - one in hid-holtek-kbd -> hid_is_usb() is called *after* hid_parse(), so
> >   report_fixup is actually called for uhid devices.
> 
> Fixed that one in my v2 patch submission.
> 
> > - another in bigbenff which is fixed by the following patch:
> > 
> > ---
> > diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c
> > index db6da21ade06..74ad8bf98bfd 100644
> > --- a/drivers/hid/hid-bigbenff.c
> > +++ b/drivers/hid/hid-bigbenff.c
> > @@ -191,7 +191,7 @@ static void bigben_worker(struct work_struct *work)
> >  		struct bigben_device, worker);
> >  	struct hid_field *report_field = bigben->report->field[0];
> > -	if (bigben->removed)
> > +	if (bigben->removed || !report_field)
> 
> I don't see how this is a "USB device" issue.  It's a good "fuzz the
> driver with invalid data" fix though :)

And to be fair, you are going to find a _LOT_ of these types of issues
in most of the HID drivers once you start fuzzing like this.  Which is
good, but independent of the USB problem.

I have seen people starting to use syzbot to create USB HID devices
(works as a loopback, so they are real), and start to fuzz the drivers.
You've had some reports already, I imagine once they realize they should
use the ids in the list from your test above, lots more issues can be
found...

thanks,

greg k-h