From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753883AbbC0IvQ (ORCPT ); Fri, 27 Mar 2015 04:51:16 -0400 Received: from mail-wi0-f182.google.com ([209.85.212.182]:35050 "EHLO mail-wi0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751638AbbC0IvM (ORCPT ); Fri, 27 Mar 2015 04:51:12 -0400 Message-ID: <551519FC.5030809@suse.cz> Date: Fri, 27 Mar 2015 09:51:08 +0100 From: Jiri Slaby User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Pan Xinhui , gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org CC: yanmin_zhang@linux.intel.com, mnipxh@gmail.com Subject: Re: [PATCH v2 RESEND] tty/n_gsm.c: fix a memory leak when gsmtty is removed References: <551111B9.3000008@intel.com> <55125E3D.2070109@intel.com> <55161530.3040103@intel.com> In-Reply-To: <55161530.3040103@intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/28/2015, 03:42 AM, Pan Xinhui wrote: > when gsmtty_remove put dlci, it will cause memory leak if dlci->port's > refcount is zero. > So we do the cleanup work in .cleanup callback instead. > > dlci will be last put in two call chains. > 1) gsmld_close -> gsm_cleanup_mux -> gsm_dlci_release -> dlci_put > 2) gsmld_remove -> dlci_put > so there is a race. the memory leak depends on the race. > > In call chain 2. we hit the memory leak. below comment tells. > > release_tty -> tty_driver_remove_tty -> gsmtty_remove -> dlci_put -> > tty_port_destructor (WARN_ON(port->itty) and return directly) > | > tty->port->itty = NULL; > | > tty_kref_put ---> release_one_tty -> gsmtty_cleanup > (added by our patch) > > So our patch fix the memory leak by doing the cleanup work after tty > core did. > > Signed-off-by: Pan Xinhui Fixes: dfabf7ffa30585 Acked-by: Jiri Slaby > --- > Changes in v2: > Don't use schedule_work to put dlci. Replace .remove with .cleanup > callback. > > drivers/tty/n_gsm.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c > index c434376..bce16e4 100644 > --- a/drivers/tty/n_gsm.c > +++ b/drivers/tty/n_gsm.c > @@ -3170,7 +3170,7 @@ static int gsmtty_break_ctl(struct tty_struct > *tty, int state) > return gsmtty_modem_update(dlci, encode); > } > > -static void gsmtty_remove(struct tty_driver *driver, struct tty_struct > *tty) > +static void gsmtty_cleanup(struct tty_struct *tty) > { > struct gsm_dlci *dlci = tty->driver_data; > struct gsm_mux *gsm = dlci->gsm; > @@ -3178,7 +3178,6 @@ static void gsmtty_remove(struct tty_driver > *driver, struct tty_struct *tty) > dlci_put(dlci); > dlci_put(gsm->dlci[0]); > mux_put(gsm); > - driver->ttys[tty->index] = NULL; > } > > /* Virtual ttys for the demux */ > @@ -3199,7 +3198,7 @@ static const struct tty_operations gsmtty_ops = { > .tiocmget = gsmtty_tiocmget, > .tiocmset = gsmtty_tiocmset, > .break_ctl = gsmtty_break_ctl, > - .remove = gsmtty_remove, > + .cleanup = gsmtty_cleanup, > }; > > -- js suse labs