Messages in this thread |  | | Subject | Re: Inconsistency in usb_add_gadget_udc_release() interface | From | Alexey Khoroshilov <> | Date | Thu, 17 Aug 2017 00:15:39 +0300 |
| |
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
|  |