Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753307AbdHQSt6 (ORCPT ); Thu, 17 Aug 2017 14:49:58 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:50836 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751665AbdHQSt5 (ORCPT ); Thu, 17 Aug 2017 14:49:57 -0400 Date: Thu, 17 Aug 2017 14:49:55 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Felipe Balbi , Alexey Khoroshilov cc: Greg Kroah-Hartman , Krzysztof Opasiak , Anton Vasilyev , USB list , Kernel development list , Subject: [PATCH] USB: Gadget core: fix inconsistency in the interface tousb_add_gadget_udc_release() In-Reply-To: <57db49b5-4276-b791-b6be-58f3b6ffde12@ispras.ru> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2422 Lines: 78 The usb_add_gadget_udc_release() routine in the USB gadget core will sometimes but not always call the gadget's release function when an error occurs. More specifically, if the struct usb_udc allocation fails then the release function is not called, and for other errors it is. As a result, users of this routine cannot know whether they need to deallocate the memory containing the gadget structure following an error. This leads to unavoidable memory leaks or double frees. This patch fixes the problem by splitting the existing device_register() call into device_initialize() and device_add(), and doing the udc allocation in between. That way, even if the allocation fails it is still possible to call device_del(), and so the release function will be always called following an error. Signed-off-by: Alan Stern Reported-by: Alexey Khoroshilov --- [as1837] drivers/usb/gadget/udc/core.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 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 @@ -1130,6 +1130,7 @@ static int check_pending_gadget_drivers( * @release: a gadget release function. * * Returns zero on success, negative errno otherwise. + * Calls the gadget release function in the latter case. */ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget, void (*release)(struct device *dev)) @@ -1137,10 +1138,6 @@ int usb_add_gadget_udc_release(struct de struct usb_udc *udc; int ret = -ENOMEM; - udc = kzalloc(sizeof(*udc), GFP_KERNEL); - if (!udc) - goto err1; - dev_set_name(&gadget->dev, "gadget"); INIT_WORK(&gadget->work, usb_gadget_state_work); gadget->dev.parent = parent; @@ -1150,7 +1147,13 @@ int usb_add_gadget_udc_release(struct de else gadget->dev.release = usb_udc_nop_release; - ret = device_register(&gadget->dev); + device_initialize(&gadget->dev); + + udc = kzalloc(sizeof(*udc), GFP_KERNEL); + if (!udc) + goto err1; + + ret = device_add(&gadget->dev); if (ret) goto err2; @@ -1197,10 +1200,10 @@ err3: device_del(&gadget->dev); err2: - put_device(&gadget->dev); kfree(udc); err1: + put_device(&gadget->dev); return ret; } EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release);