On 3/22/22 21:04, Pavel Skripkin wrote:
> syzbot reported warning in usb_submit_urb, which is caused by wrong
> endpoint type.
>
> This driver uses out bulk endpoint for communication, so
> let's check if this endpoint is present and bail out early if not.
>
> Fail log:
>
> usb 1-1: BOGUS urb xfer, pipe 3 != type 1
> WARNING: CPU: 0 PID: 4822 at drivers/usb/core/urb.c:493 usb_submit_urb+0xd27/0x1540 drivers/usb/core/urb.c:493
> Modules linked in:
> CPU: 0 PID: 4822 Comm: kworker/0:3 Tainted: G W 5.13.0-syzkaller #0
> ...
> Workqueue: usb_hub_wq hub_event
> RIP: 0010:usb_submit_urb+0xd27/0x1540 drivers/usb/core/urb.c:493
> ...
> Call Trace:
> dlfb_submit_urb+0x89/0x160 drivers/video/fbdev/udlfb.c:1969
> dlfb_set_video_mode+0x21f0/0x2950 drivers/video/fbdev/udlfb.c:315
> dlfb_ops_set_par+0x2a3/0x840 drivers/video/fbdev/udlfb.c:1110
> dlfb_usb_probe.cold+0x113e/0x1f4a drivers/video/fbdev/udlfb.c:1732
> usb_probe_interface+0x315/0x7f0 drivers/usb/core/driver.c:396
>
> Fixes: 88e58b1a42f8 ("Staging: add udlfb driver")
> Reported-and-tested-by: syzbot+53ce4a4246d0fe0fee34@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Pavel Skripkin <paskripkin@xxxxxxxxx>
applied.
Thanks,
Helge
> ---
> drivers/video/fbdev/udlfb.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
> index b9cdd02c1000..2343c7955747 100644
> --- a/drivers/video/fbdev/udlfb.c
> +++ b/drivers/video/fbdev/udlfb.c
> @@ -1649,8 +1649,9 @@ static int dlfb_usb_probe(struct usb_interface *intf,
> const struct device_attribute *attr;
> struct dlfb_data *dlfb;
> struct fb_info *info;
> - int retval = -ENOMEM;
> + int retval;
> struct usb_device *usbdev = interface_to_usbdev(intf);
> + struct usb_endpoint_descriptor *out;
>
> /* usb initialization */
> dlfb = kzalloc(sizeof(*dlfb), GFP_KERNEL);
> @@ -1664,6 +1665,12 @@ static int dlfb_usb_probe(struct usb_interface *intf,
> 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");
> + goto error;
> + }
> +
> dev_dbg(&intf->dev, "console enable=%d\n", console);
> dev_dbg(&intf->dev, "fb_defio enable=%d\n", fb_defio);
> dev_dbg(&intf->dev, "shadow enable=%d\n", shadow);
> @@ -1673,6 +1680,7 @@ static int dlfb_usb_probe(struct usb_interface *intf,
> if (!dlfb_parse_vendor_descriptor(dlfb, intf)) {
> dev_err(&intf->dev,
> "firmware not recognized, incompatible device?\n");
> + retval = -ENODEV;
> goto error;
> }
>
> @@ -1686,8 +1694,10 @@ static int dlfb_usb_probe(struct usb_interface *intf,
>
> /* allocates framebuffer driver structure, not framebuffer memory */
> info = framebuffer_alloc(0, &dlfb->udev->dev);
> - if (!info)
> + if (!info) {
> + retval = -ENOMEM;
> goto error;
> + }
>
> dlfb->info = info;
> info->par = dlfb;
