On 16.08.2017 18:24, Alan Stern wrote: > On Wed, 16 Aug 2017, Alexey Khoroshilov wrote: > >> Hello, >> >> usb_add_gadget_udc_release() gets release() argument that allows to >> release user resources. >> >> As far as I can see, the release() is called on error paths >> of usb_add_gadget_udc_release() as a result of >> put_device(&gadget->dev); >> except for the only path going via err1. >> >> As a result a caller of the usb_add_gadget_udc_release() have no chance >> to know if the release() was invoked or not. >> >> It may lead to memory leaks (drivers/usb/gadget/udc/snps_udc_core.c) >> or to double free (drivers/usb/gadget/udc/fsl_udc_core.c). >> >> Is my reading correct? If so, should we always call release() on error paths? > > How about this (untested)? > It looks reasonable. I would only suggest also to make contract description more explicit, e.g. /** * usb_add_gadget_udc_release - adds a new gadget to the udc class driver list * @parent: the parent device to this udc. Usually the controller driver's * device. * @gadget: the gadget to be added to the list. * @release: a gadget release function. * * Returns zero on success, negative errno otherwise. +* Calls the gadget release function in the latter case. */ -- Alexey Khoroshilov Linux Verification Center, ISPRAS web: http://linuxtesting.org -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
