diff mbox series

video: udlfb: Fix endpoint check

Message ID 0894f7ac-509f-435f-90ce-b55838ead65c@rowland.harvard.edu (mailing list archive)
State Accepted, archived
Headers show
Series video: udlfb: Fix endpoint check | expand

Commit Message

Alan Stern May 19, 2023, 7:32 p.m. UTC
The syzbot fuzzer detected a problem in the udlfb driver, caused by an
endpoint not having the expected type:


usb 1-1: Read EDID byte 0 failed: -71
usb 1-1: Unable to get valid EDID from device/display
------------[ cut here ]------------
usb 1-1: BOGUS urb xfer, pipe 3 != type 1
WARNING: CPU: 0 PID: 9 at drivers/usb/core/urb.c:504 usb_submit_urb+0xed6/0x1880
drivers/usb/core/urb.c:504
Modules linked in:
CPU: 0 PID: 9 Comm: kworker/0:1 Not tainted
6.4.0-rc1-syzkaller-00016-ga4422ff22142 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
04/28/2023
Workqueue: usb_hub_wq hub_event
RIP: 0010:usb_submit_urb+0xed6/0x1880 drivers/usb/core/urb.c:504
...
Call Trace:
 <TASK>
 dlfb_submit_urb+0x92/0x180 drivers/video/fbdev/udlfb.c:1980
 dlfb_set_video_mode+0x21f0/0x2950 drivers/video/fbdev/udlfb.c:315
 dlfb_ops_set_par+0x2a7/0x8d0 drivers/video/fbdev/udlfb.c:1111
 dlfb_usb_probe+0x149a/0x2710 drivers/video/fbdev/udlfb.c:1743


The current approach for this issue failed to catch the problem
because it only checks for the existence of a bulk-OUT endpoint; it
doesn't check whether this endpoint is the one that the driver will
actually use.

We can fix the problem by instead checking that the endpoint used by
the driver does exist and is bulk-OUT.

Reported-and-tested-by: syzbot+0e22d63dcebb802b9bc8@syzkaller.appspotmail.com
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Pavel Skripkin <paskripkin@gmail.com>
Fixes: aaf7dbe07385 ("video: fbdev: udlfb: properly check endpoint type")
CC: <stable@vger.kernel.org>

---

 drivers/video/fbdev/udlfb.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Helge Deller May 19, 2023, 7:51 p.m. UTC | #1
On 5/19/23 21:32, Alan Stern wrote:
> The syzbot fuzzer detected a problem in the udlfb driver, caused by an
> endpoint not having the expected type:
>
>
> usb 1-1: Read EDID byte 0 failed: -71
> usb 1-1: Unable to get valid EDID from device/display
> ------------[ cut here ]------------
> usb 1-1: BOGUS urb xfer, pipe 3 != type 1
> WARNING: CPU: 0 PID: 9 at drivers/usb/core/urb.c:504 usb_submit_urb+0xed6/0x1880
> drivers/usb/core/urb.c:504
> Modules linked in:
> CPU: 0 PID: 9 Comm: kworker/0:1 Not tainted
> 6.4.0-rc1-syzkaller-00016-ga4422ff22142 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
> 04/28/2023
> Workqueue: usb_hub_wq hub_event
> RIP: 0010:usb_submit_urb+0xed6/0x1880 drivers/usb/core/urb.c:504
> ...
> Call Trace:
>   <TASK>
>   dlfb_submit_urb+0x92/0x180 drivers/video/fbdev/udlfb.c:1980
>   dlfb_set_video_mode+0x21f0/0x2950 drivers/video/fbdev/udlfb.c:315
>   dlfb_ops_set_par+0x2a7/0x8d0 drivers/video/fbdev/udlfb.c:1111
>   dlfb_usb_probe+0x149a/0x2710 drivers/video/fbdev/udlfb.c:1743
>
>
> The current approach for this issue failed to catch the problem
> because it only checks for the existence of a bulk-OUT endpoint; it
> doesn't check whether this endpoint is the one that the driver will
> actually use.
>
> We can fix the problem by instead checking that the endpoint used by
> the driver does exist and is bulk-OUT.
>
> Reported-and-tested-by: syzbot+0e22d63dcebb802b9bc8@syzkaller.appspotmail.com
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> CC: Pavel Skripkin <paskripkin@gmail.com>
> Fixes: aaf7dbe07385 ("video: fbdev: udlfb: properly check endpoint type")
> CC: <stable@vger.kernel.org>

applied to fbdev git tree.

Thanks!
Helge

>
> ---
>
>   drivers/video/fbdev/udlfb.c |   13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
>
> Index: usb-devel/drivers/video/fbdev/udlfb.c
> ===================================================================
> --- usb-devel.orig/drivers/video/fbdev/udlfb.c
> +++ usb-devel/drivers/video/fbdev/udlfb.c
> @@ -27,6 +27,8 @@
>   #include <video/udlfb.h>
>   #include "edid.h"
>
> +#define OUT_EP_NUM	1	/* The endpoint number we will use */
> +
>   static const struct fb_fix_screeninfo dlfb_fix = {
>   	.id =           "udlfb",
>   	.type =         FB_TYPE_PACKED_PIXELS,
> @@ -1652,7 +1654,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[] = {OUT_EP_NUM + USB_DIR_OUT, 0};
>
>   	/* usb initialization */
>   	dlfb = kzalloc(sizeof(*dlfb), GFP_KERNEL);
> @@ -1666,9 +1668,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;
>   	}
>
> @@ -1927,7 +1929,8 @@ retry:
>   		}
>
>   		/* urb->transfer_buffer_length set to actual before submit */
> -		usb_fill_bulk_urb(urb, dlfb->udev, usb_sndbulkpipe(dlfb->udev, 1),
> +		usb_fill_bulk_urb(urb, dlfb->udev,
> +			usb_sndbulkpipe(dlfb->udev, OUT_EP_NUM),
>   			buf, size, dlfb_urb_completion, unode);
>   		urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>
diff mbox series

Patch

Index: usb-devel/drivers/video/fbdev/udlfb.c
===================================================================
--- usb-devel.orig/drivers/video/fbdev/udlfb.c
+++ usb-devel/drivers/video/fbdev/udlfb.c
@@ -27,6 +27,8 @@ 
 #include <video/udlfb.h>
 #include "edid.h"
 
+#define OUT_EP_NUM	1	/* The endpoint number we will use */
+
 static const struct fb_fix_screeninfo dlfb_fix = {
 	.id =           "udlfb",
 	.type =         FB_TYPE_PACKED_PIXELS,
@@ -1652,7 +1654,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[] = {OUT_EP_NUM + USB_DIR_OUT, 0};
 
 	/* usb initialization */
 	dlfb = kzalloc(sizeof(*dlfb), GFP_KERNEL);
@@ -1666,9 +1668,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;
 	}
 
@@ -1927,7 +1929,8 @@  retry:
 		}
 
 		/* urb->transfer_buffer_length set to actual before submit */
-		usb_fill_bulk_urb(urb, dlfb->udev, usb_sndbulkpipe(dlfb->udev, 1),
+		usb_fill_bulk_urb(urb, dlfb->udev,
+			usb_sndbulkpipe(dlfb->udev, OUT_EP_NUM),
 			buf, size, dlfb_urb_completion, unode);
 		urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;