From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752052AbcACQ0i (ORCPT ); Sun, 3 Jan 2016 11:26:38 -0500 Received: from mail-lb0-f174.google.com ([209.85.217.174]:36536 "EHLO mail-lb0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751595AbcACQ0g (ORCPT ); Sun, 3 Jan 2016 11:26:36 -0500 Date: Sun, 3 Jan 2016 17:27:11 +0100 From: Johan Hovold To: Mathieu OTHACEHE Cc: johan@kernel.org, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [PATCH 1/3] USB: mxu11x0: fix memory leak on usb_serial private data Message-ID: <20160103162711.GD25304@localhost> References: <1451831161-26792-1-git-send-email-m.othacehe@gmail.com> <1451831161-26792-2-git-send-email-m.othacehe@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1451831161-26792-2-git-send-email-m.othacehe@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Jan 03, 2016 at 03:25:59PM +0100, Mathieu OTHACEHE wrote: > On nominal execution, private data allocated on port_probe and attach > are never freed. Add port_remove and release callbacks to free them > respectively. Ouch. I thought I'd vetted the driver for further memleaks but apparently missed the most obvious ones. > Signed-off-by: Mathieu OTHACEHE > --- > drivers/usb/serial/mxu11x0.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/usb/serial/mxu11x0.c b/drivers/usb/serial/mxu11x0.c > index e3c3f57c..0fe7eab 100644 > --- a/drivers/usb/serial/mxu11x0.c > +++ b/drivers/usb/serial/mxu11x0.c > @@ -328,6 +328,14 @@ static int mxu1_download_firmware(struct usb_serial *serial, > return 0; > } > > +static void mxu1_release(struct usb_serial *serial) > +{ > + struct mxu1_device *mxdev; > + > + mxdev = usb_get_serial_data(serial); > + kfree(mxdev); > +} Please place this once after the matching attach (startup) callback. > + > static int mxu1_port_probe(struct usb_serial_port *port) > { > struct mxu1_port *mxport; > @@ -368,6 +376,16 @@ static int mxu1_port_probe(struct usb_serial_port *port) > return 0; > } > > +static int mxu1_port_remove(struct usb_serial_port *port) > +{ > + struct mxu1_port *mxport; > + > + mxport = usb_get_serial_port_data(port); > + kfree(mxport); > + > + return 0; > +} > + > static int mxu1_startup(struct usb_serial *serial) > { > struct mxu1_device *mxdev; Thanks, Johan