To recap: The problem is that uvcvideo tries to change an interface altsetting (from within uvc_video_start_streaming -> uvc_video_start_transfer) after the driver has been unbound from the device. This triggers an invalid memory reference. On Sun, Nov 22, 2020 at 08:03:43PM +0000, John Boero wrote: > Thanks Alan > I just spent some more time investigating and playing with different patches, > locks, mutexes, and sleeps, and I think I see exactly what's happening now. > I now understand why it: > A) seems to happen randomly during uvc start_stream > B) affects multiple device vendors > C) has been reported in RaspberryPi and IoT threads > > I put a trace on usb/core/hub.c:usb_disconnect to identify why the device was > disconnecting and it seems this is a low power issue. An idle webcam appears > fine to usbcore but as soon as you initialize it or uvc starts a stream, it > consumes more power, might find the cable can't supply it, and then disconnects > via interrupt. In my case I can reproduce this consistently with a cheap USB > extension cable, but this issue appears common to passive hubs, and IoT or SBCs > that don't always supply clean power over USB. My simplified patch can at least > protect usbcore from crashing on a bad device: > > From 73019d79fe4fd8b2c945005f8a067f528d8056fd Mon Sep 17 00:00:00 2001 > From: jboero <boeroboy@xxxxxxxxx> > Date: Sun, 22 Nov 2020 19:30:41 +0000 > Subject: [PATCH] USBCore NULL interface deref fix > > --- > drivers/usb/core/usb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c > index bafc113f2b3e..da46c84c87ce 100644 > --- a/drivers/usb/core/usb.c > +++ b/drivers/usb/core/usb.c > @@ -278,7 +278,7 @@ struct usb_interface *usb_ifnum_to_if(const struct > usb_device *dev, > if (!config) > return NULL; > for (i = 0; i < config->desc.bNumInterfaces; i++) > - if (config->interface[i]->altsetting[0] > + if (config->interface[i] && config->interface[i]->altsetting[0] > .desc.bInterfaceNumber == ifnum) > return config->interface[i]; I really dislike the idea of papering over a problem instead of fixing it properly. > This protects ongoing USB functionality including lsusb, and prevents a hang on > reboot after error. It doesn't help a user diagnose the error on the UVC side. > A fix from the uvc side is a little trickier and I'd like an opinion on how > best to handle locks in uvc_video_start_transfer. I've tried a few options > around uvcvideo.c:1874 > > ret = usb_set_interface(stream->dev->udev, intfnum, altsetting); > > I've even used multiple msleeps and checked for NULL interfaces but that feels > like a cheap trick and I was wondering what lock solution might help best here? Laurent Pinchart is the person to ask. He is the maintainer of the uvcvideo driver. I know very little about it. Alan Stern
