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 for Android: free password hash cracker in your pocket

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

Message-ID: <20190507150725.GB29016@localhost>
Date:   Tue, 7 May 2019 17:07:25 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     Johan Hovold <johan@...nel.org>,
        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 Tue, May 07, 2019 at 10:42:58AM -0400, Alan Stern wrote:
> On Tue, 7 May 2019, Johan Hovold wrote:

> > > @@ -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.
> 
> Thanks for the review.  You're right about the memory leak (although 
> you're wrong about the later error paths: smsusb_term_device() 
> deallocates dev).

Indeed, I missed the free in smsusb_term_device(). Sorry about that.

Johan