linux-kernel - Re: general protection fault in smsusb_init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190507083430.GD4333@localhost>
Date:   Tue, 7 May 2019 10:34:30 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     syzbot <syzbot+53f029db71c19a47325a@...kaller.appspotmail.com>,
        andreyknvl@...gle.com, linux-kernel@...r.kernel.org,
        linux-media@...r.kernel.org, linux-usb@...r.kernel.org,
        mchehab@...nel.org, syzkaller-bugs@...glegroups.com,
        wen.yang99@....com.cn
Subject: Re: general protection fault in smsusb_init_device

On Mon, May 06, 2019 at 04:41:41PM -0400, Alan Stern wrote:
> On Thu, 18 Apr 2019, syzbot wrote:
> 
> > Hello,
> > 
> > syzbot found the following crash on:
> > 
> > HEAD commit:    d34f9519 usb-fuzzer: main usb gadget fuzzer driver
> > git tree:       https://github.com/google/kasan/tree/usb-fuzzer
> > console output: https://syzkaller.appspot.com/x/log.txt?x=128ec3fd200000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=c73d1bb5aeaeae20
> > dashboard link: https://syzkaller.appspot.com/bug?extid=53f029db71c19a47325a
> > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=16138e67200000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=128dddbf200000
> > 
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+53f029db71c19a47325a@...kaller.appspotmail.com
> > 
> > usb 1-1: config 0 descriptor??
> > usb 1-1: string descriptor 0 read error: -71
> > smsusb:smsusb_probe: board id=18, interface number 0
> > kasan: CONFIG_KASAN_INLINE enabled
> > kasan: GPF could be caused by NULL-ptr deref or user memory access
> > general protection fault: 0000 [#1] SMP KASAN PTI
> > CPU: 1 PID: 22 Comm: kworker/1:1 Not tainted 5.1.0-rc5-319617-gd34f951 #4
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
> > Google 01/01/2011
> > Workqueue: usb_hub_wq hub_event
> > RIP: 0010:smsusb_init_device+0x366/0x937  
> > drivers/media/usb/siano/smsusb.c:429
> 
> The driver assumes endpoint 1in exists, and doesn't check the existence 
> of the endpoints it uses.
> 
> Alan Stern
> 
> 
> #syz test: https://github.com/google/kasan.git usb-fuzzer
> 
>  drivers/media/usb/siano/smsusb.c |   32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)
> 
> Index: usb-devel/drivers/media/usb/siano/smsusb.c
> ===================================================================
> --- usb-devel.orig/drivers/media/usb/siano/smsusb.c
> +++ usb-devel/drivers/media/usb/siano/smsusb.c
> @@ -400,6 +400,7 @@ static int smsusb_init_device(struct usb
>  	struct smsusb_device_t *dev;
>  	void *mdev;
>  	int i, rc;
> +	int in_maxp;
>  
>  	/* create device object */
>  	dev = kzalloc(sizeof(struct smsusb_device_t), GFP_KERNEL);
> @@ -411,6 +412,23 @@ static int smsusb_init_device(struct usb
>  	dev->udev = interface_to_usbdev(intf);
>  	dev->state = SMSUSB_DISCONNECTED;
>  
> +	for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
> +		struct usb_endpoint_descriptor *desc =
> +				&intf->cur_altsetting->endpoint[i].desc;
> +
> +		if (desc->bEndpointAddress & USB_DIR_IN) {
> +			dev->in_ep = desc->bEndpointAddress;
> +			in_maxp = usb_endpoint_maxp(desc);
> +		} else {
> +			dev->out_ep = desc->bEndpointAddress;
> +		}
> +	}
> +
> +	pr_debug("in_ep = %02x, out_ep = %02x\n",
> +		dev->in_ep, dev->out_ep);
> +	if (!dev->in_ep || !dev->out_ep)	/* Missing endpoints? */
> +		return -EINVAL;

Looks like you're now leaking dev here, and so is the current code in
the later error paths.

Since this return value will be returned from probe, you may want to use
-ENXIO or -ENODEV instead of -EINVAL.

Looks good otherwise.

Johan