linux-kernel - Re: Inconsistency in usb_add_gadget_udc

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 17 Aug 2017 00:15:39 +0300
From:   Alexey Khoroshilov <khoroshilov@...ras.ru>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Krzysztof Opasiak <k.opasiak@...sung.com>,
        Anton Vasilyev <vasilyev@...ras.ru>, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, ldv-project@...uxtesting.org
Subject: Re: Inconsistency in usb_add_gadget_udc_release() interface

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