From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DA925C43219 for ; Tue, 30 Apr 2019 15:14:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AB83120835 for ; Tue, 30 Apr 2019 15:14:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726244AbfD3POz (ORCPT ); Tue, 30 Apr 2019 11:14:55 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:52827 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725942AbfD3POz (ORCPT ); Tue, 30 Apr 2019 11:14:55 -0400 Received: from 162-237-133-238.lightspeed.rcsntx.sbcglobal.net ([162.237.133.238] helo=lindsey) by youngberry.canonical.com with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1hLUSx-00068L-0T; Tue, 30 Apr 2019 15:14:43 +0000 Date: Tue, 30 Apr 2019 10:14:37 -0500 From: Tyler Hicks To: "Tobin C. Harding" Cc: "David S. Miller" , Greg Kroah-Hartman , Andy Shevchenko , Ido Schimmel , Alexander Duyck , Florian Fainelli , Wang Hai , YueHaibing , Amritha Nambiar , Dmitry Torokhov , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] bridge: Fix error path for kobject_init_and_add() Message-ID: <20190430151437.GA13709@lindsey> References: <20190430002817.10785-1-tobin@kernel.org> <20190430002817.10785-2-tobin@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190430002817.10785-2-tobin@kernel.org> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2019-04-30 10:28:15, Tobin C. Harding wrote: > Currently error return from kobject_init_and_add() is not followed by a > call to kobject_put(). This means there is a memory leak. > > Add call to kobject_put() in error path of kobject_init_and_add(). > > Signed-off-by: Tobin C. Harding > --- > net/bridge/br_if.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c > index 41f0a696a65f..e5c8c9941c51 100644 > --- a/net/bridge/br_if.c > +++ b/net/bridge/br_if.c > @@ -607,8 +607,10 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, > > err = kobject_init_and_add(&p->kobj, &brport_ktype, &(dev->dev.kobj), > SYSFS_BRIDGE_PORT_ATTR); > - if (err) > + if (err) { > + kobject_put(&p->kobj); > goto err1; > + } I think this is duplicating the code under the err2 label and doing so in a way that would introduce a double free. If the refcount hits 0 in the kobject_put(), release_nbp() is called which calls kfree() on the p pointer. However, the p pointer is never set to NULL like what's done under the err2 label. Once we're back in br_add_if(), kfree() is called on the p pointer again just before returning. I think it would be better if you just jumped to the err2 label instead of err1. err1 will no longer be used so, unfortunately, you'll have to refactor all the labels at the same time. Tyler > > err = br_sysfs_addif(p); > if (err) > -- > 2.21.0 >