Messages in this thread | | | Date | Fri, 19 May 2023 12:38:15 +0200 | Subject | Re: [syzbot] [fbdev?] [usb?] WARNING in dlfb_submit_urb/usb_submit_urb (2) | From | Helge Deller <> |
| |
On 5/18/23 22:35, Alan Stern wrote: > On Thu, May 18, 2023 at 09:06:12PM +0200, Helge Deller wrote: >> * Alan Stern <stern@rowland.harvard.edu>: >>> On Thu, May 18, 2023 at 04:16:33PM +0200, Helge Deller wrote: >>>> On 5/18/23 15:54, Alan Stern wrote: >>>>> In this case it looks like dlfb_usb_probe() or one of the routines it >>>>> calls is wrong; it assumes that an endpoint has the expected type >>>>> without checking. More precisely, it thinks an endpoint is BULK when >>>>> actually it is INTERRUPT. That's what needs to be fixed. >>>> >>>> Maybe usb_submit_urb() should return an error so that drivers can >>>> react on it, instead of adding the same kind of checks to all drivers? >>> >>> Feel free to submit a patch doing this. >> >> As you wrote above, this may break other drivers too, so I'd leave that >> discussion & decision to the USB maintainers (like you). >> >>> But the checks should be added >>> in any case; without them the drivers are simply wrong. >> >> I pushed the hackish patch below through the syz tests which gives this log: >> (see https://syzkaller.appspot.com/text?tag=CrashLog&x=160b7509280000) >> [ 77.559566][ T9] usb 1-1: Unable to get valid EDID from device/display >> [ 77.587021][ T9] WARNING: BOGUS urb xfer, pipe 3 != type 1 (fix driver to choose correct endpoint) >> [ 77.596448][ T9] usb 1-1: dlfb_urb_completion - nonzero write bulk status received: -115 >> [ 77.605308][ T9] usb 1-1: submit urb error: -22 >> [ 77.613225][ T9] udlfb: probe of 1-1:0.52 failed with error -22 >> >> So, basically there is no urgent fix needed for the dlfb fbdev driver, >> as it will gracefully fail as is (which is correct). >> >> What do you suggest we should do with this syzkaller-bug ? >> I'd rate it as false-alarm, but it will continue to complain because of >> the dev_WARN() in urb.c > > Let's try this patch instead. It might contain a stupid error because I > haven't even tried to compile it, but it ought to fix the real problem.
Patch looks good and survived the test.
Will you send a proper patch to the fbdev mailing list, so that I can include it?
Helge
> > #syz test: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git a4422ff22142 > > Index: usb-devel/drivers/video/fbdev/udlfb.c > =================================================================== > --- usb-devel.orig/drivers/video/fbdev/udlfb.c > +++ usb-devel/drivers/video/fbdev/udlfb.c > @@ -1652,7 +1652,7 @@ static int dlfb_usb_probe(struct usb_int > struct fb_info *info; > int retval; > struct usb_device *usbdev = interface_to_usbdev(intf); > - struct usb_endpoint_descriptor *out; > + static u8 out_ep[] = {1 + USB_DIR_OUT, 0}; > > /* usb initialization */ > dlfb = kzalloc(sizeof(*dlfb), GFP_KERNEL); > @@ -1666,9 +1666,9 @@ static int dlfb_usb_probe(struct usb_int > dlfb->udev = usb_get_dev(usbdev); > usb_set_intfdata(intf, dlfb); > > - retval = usb_find_common_endpoints(intf->cur_altsetting, NULL, &out, NULL, NULL); > - if (retval) { > - dev_err(&intf->dev, "Device should have at lease 1 bulk endpoint!\n"); > + if (!usb_check_bulk_endpoints(intf, out_ep)) { > + dev_err(&intf->dev, "Invalid DisplayLink device!\n"); > + retval = -EINVAL; > goto error; > } >
| |