Re: [PATCH] usb: core: Null deref in kernel with USB webcams.

[John Boero]

Sorry header was generated by git email and I should have
paid closer attention to it before sending.
Long time listener, first time caller.

Yes the patch is backwards sorry.  Testing alt proposal from
stern@xxxxxxxxxxxxxxxxxxx.  It may be a buggy driver
but it would be nice if a buggy driver couldn't bring down
the entire usb core. lsusb hangs until reboot or reset of usb.

It seems to behave fine on first use.  Run Zoom or cheese
works fine first time.  Subsequent runs, no device found
and usb is crashed with trace in dmesg.

Thanks
John


On Thu, Nov 12, 2020 at 5:04 PM Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Nov 12, 2020 at 03:52:02PM +0000, John Boero wrote:
> > >From 54f9886454e9a28e8d943c1cef15df9c11555df7 Mon Sep 17 00:00:00 2001
> > From: JohnnyB <jboero@xxxxxxxxxxxxxxxxxxxxxxxx>
>
> Why all this header here?
>
> And the from: line doesn't match your Signed-off-by: line :(
>
> > Date: Thu, 12 Nov 2020 15:28:29 +0000
> > Subject: [PATCH] usb: core: Null deref in kernel with USB webcams.
> >
> > Fixes: Ubuntu Launchpad bug 1827452
> >
> > This is my first attempt at a kernel contribution so sorry if sloppy.
>
> No need to put this in the changelog text and have it be in the kernel
> for foever :)
>
> >
> > There is some kind of race condition affecting Logitech
> > webcams that crash USB with a null dereference.
> > Affects raspberry pi devices as well as x86.
> > No check on dev before dereference.
> > Simple fix for issue experienced for months in
> > both x86 and arm/rpi environments.
> >
> > Signed-off-by: John Boero <boeroboy@xxxxxxxxx>
> >
> > ---
> > drivers/usb/core/usb.c | 6 +-----
> > 1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index d8756ffe513a..9b4ac4415f1a 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -272,13 +272,9 @@ EXPORT_SYMBOL_GPL(usb_find_alt_setting);
> > struct usb_interface *usb_ifnum_to_if(const struct usb_device *dev,
> >                                      unsigned ifnum)
> > {
> > -       struct usb_host_config *config = NULL;
> > +       struct usb_host_config *config = dev->actconfig;
> >        int i;
> >
> > -       if (!dev)
> > -               return NULL;
> > -
> > -       config = dev->actconfig;
> >        if (!config)
> >                return NULL;
> >        for (i = 0; i < config->desc.bNumInterfaces; i++)
>
> This patch is corrupted and can not be applied, but also, it looks
> backwards, right?
>
> And how about we find the race condition and fix that instead of trying
> to paper over it here?
>
> thanks,
>
> greg k-h