Re: general protection fault in smsusb_init_device

[Johan Hovold <johan@kernel.org>]

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