Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752524AbdHPVPn (ORCPT ); Wed, 16 Aug 2017 17:15:43 -0400 Received: from mail.ispras.ru ([83.149.199.45]:42144 "EHLO mail.ispras.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752465AbdHPVPl (ORCPT ); Wed, 16 Aug 2017 17:15:41 -0400 Subject: Re: Inconsistency in usb_add_gadget_udc_release() interface To: Alan Stern Cc: Felipe Balbi , Greg Kroah-Hartman , Krzysztof Opasiak , Anton Vasilyev , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, ldv-project@linuxtesting.org References: From: Alexey Khoroshilov Message-ID: <57db49b5-4276-b791-b6be-58f3b6ffde12@ispras.ru> Date: Thu, 17 Aug 2017 00:15:39 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1333 Lines: 43 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