On Wed, Jan 03, 2018 at 12:51:51PM -0500, Alan Stern wrote: > The error-handling pathways in usb_add_gadget_udc_release() are messed > up. Aside from the uninformative statement labels, they can deallocate > the udc structure after calling put_device(), which is a double-free. > This was observed by KASAN in automatic testing. > > This patch cleans up the routine. It preserves the requirement that > when any failure occurs, we call put_device(&gadget->dev). > > Signed-off-by: Alan Stern <[email protected]> > Reported-by: Fengguang Wu <[email protected]> > CC: <[email protected]> > > --- > > Not having received any feedback on this patch for a while, I have > decided to submit it. The problem it fixes is quite obviously a real > one, when you look at the existing code. > > Alan Stern > > > [as1856] > > > drivers/usb/gadget/udc/core.c | 28 +++++++++++++--------------- > 1 file changed, 13 insertions(+), 15 deletions(-) > > Index: usb-4.x/drivers/usb/gadget/udc/core.c > =================================================================== > --- usb-4.x.orig/drivers/usb/gadget/udc/core.c > +++ usb-4.x/drivers/usb/gadget/udc/core.c > @@ -1147,11 +1147,7 @@ int usb_add_gadget_udc_release(struct de > > udc = kzalloc(sizeof(*udc), GFP_KERNEL); > if (!udc) > - goto err1; > - > - ret = device_add(&gadget->dev); > - if (ret) > - goto err2; > + goto err_put_gadget; > > device_initialize(&udc->dev); > udc->dev.release = usb_udc_release; > @@ -1160,7 +1156,11 @@ int usb_add_gadget_udc_release(struct de > udc->dev.parent = parent; > ret = dev_set_name(&udc->dev, "%s", kobject_name(&parent->kobj)); > if (ret) > - goto err3; > + goto err_put_udc; > + > + ret = device_add(&gadget->dev); > + if (ret) > + goto err_put_udc; > > udc->gadget = gadget; > gadget->udc = udc; > @@ -1170,7 +1170,7 @@ int usb_add_gadget_udc_release(struct de > > ret = device_add(&udc->dev); > if (ret) > - goto err4; > + goto err_unlist_udc; > > usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED); > udc->vbus = true; > @@ -1178,27 +1178,25 @@ int usb_add_gadget_udc_release(struct de > /* pick up one of pending gadget drivers */ > ret = check_pending_gadget_drivers(udc); > if (ret) > - goto err5; > + goto err_del_udc; > > mutex_unlock(&udc_lock); > > return 0; > > -err5: > + err_del_udc: > device_del(&udc->dev); > > -err4: > + err_unlist_udc: > list_del(&udc->list); > mutex_unlock(&udc_lock); > > -err3: > - put_device(&udc->dev); > device_del(&gadget->dev); > > -err2: > - kfree(udc); > + err_put_udc: > + put_device(&udc->dev); > > -err1: > + err_put_gadget: > put_device(&gadget->dev); > return ret; > } >
Reviewed-by: Peter Chen <[email protected]> -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
