Re: general protection fault in go7007_usb_probe

From: Oliver Neukum
Date: Thu Apr 30 2020 - 09:10:18 EST


Am Dienstag, den 21.04.2020, 16:36 -0700 schrieb syzbot:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: e9010320 usb: cdns3: gadget: make a bunch of functions sta..
> git tree: https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=1263a930100000
> kernel config: https://syzkaller.appspot.com/x/.config?x=bd14feb44652cfaf
> dashboard link: https://syzkaller.appspot.com/bug?extid=cabfa4b5b05ff6be4ef0
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+cabfa4b5b05ff6be4ef0@xxxxxxxxxxxxxxxxxxxxxxxxx

#syz test: https://github.com/google/kasan.git e9010320
From a006a724881bfb592db61ba17db1e217b9615cc4 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@xxxxxxxx>
Date: Wed, 22 Apr 2020 13:49:55 +0200
Subject: [PATCH] go7007: add only insanity checking

A malicious USB device may lack endpoints the driver assumes to exist
Accessing them leads to NULL pointer accesses. This patch introduces
sanity checking.

Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
Fixes: 866b8695d67e8 ("Staging: add the go7007 video driver")
---
drivers/media/usb/go7007/go7007-usb.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/go7007/go7007-usb.c b/drivers/media/usb/go7007/go7007-usb.c
index f889c9d740cd..a45af149cadc 100644
--- a/drivers/media/usb/go7007/go7007-usb.c
+++ b/drivers/media/usb/go7007/go7007-usb.c
@@ -1132,6 +1132,20 @@ static int go7007_usb_probe(struct usb_interface *intf,
go->hpi_ops = &go7007_usb_onboard_hpi_ops;
go->hpi_context = usb;

+ if (!usb->usbdev) {
+ printk(KERN_ERR"It is full of stars!\n");
+ BUG();
+ }
+
+ if (!usb->usbdev->ep_in) {
+ printk(KERN_ERR"It is also full of stars!\n");
+ BUG();
+ }
+
+ ep = usb->usbdev->ep_in[4];
+ if (!ep)
+ return -ENODEV;
+
/* Allocate the URB and buffer for receiving incoming interrupts */
usb->intr_urb = usb_alloc_urb(0, GFP_KERNEL);
if (usb->intr_urb == NULL)
@@ -1141,7 +1155,6 @@ static int go7007_usb_probe(struct usb_interface *intf,
if (usb->intr_urb->transfer_buffer == NULL)
goto allocfail;

- ep = usb->usbdev->ep_in[4];
if (usb_endpoint_type(&ep->desc) == USB_ENDPOINT_XFER_BULK)
usb_fill_bulk_urb(usb->intr_urb, usb->usbdev,
usb_rcvbulkpipe(usb->usbdev, 4),
@@ -1263,9 +1276,13 @@ static int go7007_usb_probe(struct usb_interface *intf,

/* Allocate the URBs and buffers for receiving the video stream */
if (board->flags & GO7007_USB_EZUSB) {
+ if (!usb->usbdev->ep_in[6])
+ goto allocfail;
v_urb_len = 1024;
video_pipe = usb_rcvbulkpipe(usb->usbdev, 6);
} else {
+ if (!usb->usbdev->ep_in[1])
+ goto allocfail;
v_urb_len = 512;
video_pipe = usb_rcvbulkpipe(usb->usbdev, 1);
}
@@ -1285,6 +1302,8 @@ static int go7007_usb_probe(struct usb_interface *intf,
/* Allocate the URBs and buffers for receiving the audio stream */
if ((board->flags & GO7007_USB_EZUSB) &&
(board->main_info.flags & GO7007_BOARD_HAS_AUDIO)) {
+ if (!usb->usbdev->ep_in[8])
+ goto allocfail;
for (i = 0; i < 8; ++i) {
usb->audio_urbs[i] = usb_alloc_urb(0, GFP_KERNEL);
if (usb->audio_urbs[i] == NULL)
--
2.16.4