Message ID | 20230208125758.1515806-1-gregkh@linuxfoundation.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp3444985wrn; Wed, 8 Feb 2023 05:08:36 -0800 (PST) X-Google-Smtp-Source: AK7set+TRSNK782091E92ubqx/m+lawVEcOYw4vUqGSjZrjE/CG9GYs3jWt+P8e0ACK9cVAz0Svv X-Received: by 2002:a17:906:51d7:b0:887:dbe1:25ac with SMTP id v23-20020a17090651d700b00887dbe125acmr6480407ejk.0.1675861716423; Wed, 08 Feb 2023 05:08:36 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675861716; cv=none; d=google.com; s=arc-20160816; b=yhfPWsZZcYCLKPXGA8xZzRbjxY16rfMoAQ59e5Dmfq1W25i+L1rCfhzmsB9LsUZ3jD qAmH2Da+zjWSJr6JRWxQ6mrdDl3PwVxrFcFnmDH3hGofhVHoTZOG67zxYm3PA/R5do9s aFTMd3Y6NgPHET/WkuDTXExtbQnNK9HZ3ifPL5sWj5u2UhoUKTY81dYnxl1pTdBmBRm6 YQzxtUkBYC0vIbHonrddUC/kxZnjKfCxfJciFMampnQvijfNyf5DiG03f6UHBHtI9qD5 6B7AWcw4RIy1E9eDPkUHfUJ61iEeG+khDl9qR2RafpRWnbEtQXNE+F6b1eVymhIsd41x lwvQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=QC92YlV4U+EZVUA0rMLUOhgMxcB6ZDx5DL8Lrb33Zco=; b=dsY6TX6GrL/EjqJZolORr6/2H3f+NY6sRkHwYP+dUSk5BC7MOI1hekSeBzXwTQdhYj 1qLYNAY+iXITJne3q1U6/l8sLX9Fb9o7765zlSbFfkc1JXoTQw8vYyidAPFyT+dh1Ws4 vJFX1xlFX2bcu1pduvznAOJutHcj4j/vf1hbPQjcMbLp1WgpoUlNjuTndJNNmqqRe60N QQv1PzT3cQMzCPlSBAVLeguXhzgsmiTEOCOlCglMC9bwJ3egBweKgiBWkGgxR1n48DfH D0xBpAKmRl3/axWNmYowet9H+uvgGeGSC64quxusdKdRh6jl9yYukDf354eudrSNKrTe 8Z/Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=rIlTvocD; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id mu31-20020a1709068a9f00b00883736a7a67si18083500ejc.530.2023.02.08.05.08.13; Wed, 08 Feb 2023 05:08:36 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=rIlTvocD; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229648AbjBHM6J (ORCPT <rfc822;ivan.orlov0322@gmail.com> + 99 others); Wed, 8 Feb 2023 07:58:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49728 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231324AbjBHM6G (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 8 Feb 2023 07:58:06 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D0D507DB4 for <linux-kernel@vger.kernel.org>; Wed, 8 Feb 2023 04:58:04 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 6472C616CE for <linux-kernel@vger.kernel.org>; Wed, 8 Feb 2023 12:58:04 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4FCACC4339B; Wed, 8 Feb 2023 12:58:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1675861083; bh=VrbCzxOmy3ybuwWlA7ZbkY+1uA2a/fPo2GEiHFqaUl0=; h=From:To:Cc:Subject:Date:From; b=rIlTvocDgkpF8y33io5K1nKHrLhuLFhEc02ASgrKCdZaxXRVEKswfLAiqe/ugm03u 895TSS/cqANm0zLs0qn/ySYsi6wiWqwHRhTEYr/ibwFTwrfMOPJHZdbOFSy6gy29la 8GQgrLZaBruG/XIR7MltHmqrBUos2sb54QbBoDWY= From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Tudor Ambarus <tudor.ambarus@microchip.com>, Pratyush Yadav <pratyush@kernel.org>, Michael Walle <michael@walle.cc>, Miquel Raynal <miquel.raynal@bootlin.com>, Richard Weinberger <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, linux-mtd@lists.infradead.org, stable <stable@kernel.org> Subject: [PATCH v2] mtd: spi-nor: fix memory leak when using debugfs_lookup() Date: Wed, 8 Feb 2023 13:57:58 +0100 Message-Id: <20230208125758.1515806-1-gregkh@linuxfoundation.org> X-Mailer: git-send-email 2.39.1 MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=1182; i=gregkh@linuxfoundation.org; h=from:subject; bh=VrbCzxOmy3ybuwWlA7ZbkY+1uA2a/fPo2GEiHFqaUl0=; b=owGbwMvMwCRo6H6F97bub03G02pJDMmP54Ryek+ZorD9hV7p6l9CjqdzVNf0XSpOMHpxnaF/7ZOJ zNxzOmJZGASZGGTFFFm+bOM5ur/ikKKXoe1pmDmsTCBDGLg4BWAiq3czzNNu1r/I6+bHMdeX6U9VjZ IO22RTdYYFl/3mfCr3X3LS9eLdeym7n8tLr/x3CgA= X-Developer-Key: i=gregkh@linuxfoundation.org; a=openpgp; fpr=F4B60CC5BF78C2214A313DCB3147D40DDB2DFB29 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1757268366366522628?= X-GMAIL-MSGID: =?utf-8?q?1757268375178900400?= |
Series |
[v2] mtd: spi-nor: fix memory leak when using debugfs_lookup()
|
|
Commit Message
Greg KH
Feb. 8, 2023, 12:57 p.m. UTC
When calling debugfs_lookup() the result must have dput() called on it,
otherwise the memory will leak over time.
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>
Cc: Pratyush Yadav <pratyush@kernel.org>
Cc: Michael Walle <michael@walle.cc>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2: fix up to work when module is removed and added, making the fix
much simpler.
drivers/mtd/spi-nor/debugfs.c | 1 +
1 file changed, 1 insertion(+)
Comments
Am 2023-02-08 13:57, schrieb Greg Kroah-Hartman: > When calling debugfs_lookup() the result must have dput() called on it, > otherwise the memory will leak over time. > > Cc: Tudor Ambarus <tudor.ambarus@microchip.com> > Cc: Pratyush Yadav <pratyush@kernel.org> > Cc: Michael Walle <michael@walle.cc> > Cc: Miquel Raynal <miquel.raynal@bootlin.com> > Cc: Richard Weinberger <richard@nod.at> > Cc: Vignesh Raghavendra <vigneshr@ti.com> > Cc: linux-mtd@lists.infradead.org > Cc: stable <stable@kernel.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > v2: fix up to work when module is removed and added, making the fix > much simpler. > > drivers/mtd/spi-nor/debugfs.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mtd/spi-nor/debugfs.c > b/drivers/mtd/spi-nor/debugfs.c > index ff895f6758ea..af41fbc09a97 100644 > --- a/drivers/mtd/spi-nor/debugfs.c > +++ b/drivers/mtd/spi-nor/debugfs.c > @@ -242,6 +242,7 @@ void spi_nor_debugfs_register(struct spi_nor *nor) > > d = debugfs_create_dir(dev_name(nor->dev), rootdir); > nor->debugfs_root = d; > + dput(rootdir); rootdir might either be the return value of debugfs_lookup() or debugfs_create_dir(). dput() is probably wrong for the latter, right? Also there is an early return, where the dput() is missing, too. -michael
On Wed, Feb 08, 2023 at 02:36:23PM +0100, Michael Walle wrote: > Am 2023-02-08 13:57, schrieb Greg Kroah-Hartman: > > When calling debugfs_lookup() the result must have dput() called on it, > > otherwise the memory will leak over time. > > > > Cc: Tudor Ambarus <tudor.ambarus@microchip.com> > > Cc: Pratyush Yadav <pratyush@kernel.org> > > Cc: Michael Walle <michael@walle.cc> > > Cc: Miquel Raynal <miquel.raynal@bootlin.com> > > Cc: Richard Weinberger <richard@nod.at> > > Cc: Vignesh Raghavendra <vigneshr@ti.com> > > Cc: linux-mtd@lists.infradead.org > > Cc: stable <stable@kernel.org> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > v2: fix up to work when module is removed and added, making the fix > > much simpler. > > > > drivers/mtd/spi-nor/debugfs.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/mtd/spi-nor/debugfs.c > > b/drivers/mtd/spi-nor/debugfs.c > > index ff895f6758ea..af41fbc09a97 100644 > > --- a/drivers/mtd/spi-nor/debugfs.c > > +++ b/drivers/mtd/spi-nor/debugfs.c > > @@ -242,6 +242,7 @@ void spi_nor_debugfs_register(struct spi_nor *nor) > > > > d = debugfs_create_dir(dev_name(nor->dev), rootdir); > > nor->debugfs_root = d; > > + dput(rootdir); > > rootdir might either be the return value of debugfs_lookup() or > debugfs_create_dir(). dput() is probably wrong for the latter, > right? Also there is an early return, where the dput() is missing, > too. {sigh} Yeah, this is all wrong, sorry. Let me fix this up again, properly. And to do it properly, let's have the module remove the directory if it is unloaded, like a good module should :) greg k-h
Am 2023-02-08 15:12, schrieb Greg Kroah-Hartman: > On Wed, Feb 08, 2023 at 02:36:23PM +0100, Michael Walle wrote: >> Am 2023-02-08 13:57, schrieb Greg Kroah-Hartman: >> > When calling debugfs_lookup() the result must have dput() called on it, >> > otherwise the memory will leak over time. >> > >> > Cc: Tudor Ambarus <tudor.ambarus@microchip.com> >> > Cc: Pratyush Yadav <pratyush@kernel.org> >> > Cc: Michael Walle <michael@walle.cc> >> > Cc: Miquel Raynal <miquel.raynal@bootlin.com> >> > Cc: Richard Weinberger <richard@nod.at> >> > Cc: Vignesh Raghavendra <vigneshr@ti.com> >> > Cc: linux-mtd@lists.infradead.org >> > Cc: stable <stable@kernel.org> >> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> > --- >> > v2: fix up to work when module is removed and added, making the fix >> > much simpler. >> > >> > drivers/mtd/spi-nor/debugfs.c | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/drivers/mtd/spi-nor/debugfs.c >> > b/drivers/mtd/spi-nor/debugfs.c >> > index ff895f6758ea..af41fbc09a97 100644 >> > --- a/drivers/mtd/spi-nor/debugfs.c >> > +++ b/drivers/mtd/spi-nor/debugfs.c >> > @@ -242,6 +242,7 @@ void spi_nor_debugfs_register(struct spi_nor *nor) >> > >> > d = debugfs_create_dir(dev_name(nor->dev), rootdir); >> > nor->debugfs_root = d; >> > + dput(rootdir); >> >> rootdir might either be the return value of debugfs_lookup() or >> debugfs_create_dir(). dput() is probably wrong for the latter, >> right? Also there is an early return, where the dput() is missing, >> too. > > {sigh} > > Yeah, this is all wrong, sorry. Let me fix this up again, properly. > And to do it properly, let's have the module remove the directory if it > is unloaded, like a good module should :) There were some complications. IIRC you'd need to do reference counting, to determine whether you are the last user of the rootdir. Other subsys create an empty rootdir in their .init(). But that was hard to do in MTD. Again memory is hazy.. Therefore, I resorted to create it on the fly if there isn't already one. Maybe you got some better/simpler idea :) -michael
On Wed, Feb 08, 2023 at 03:24:31PM +0100, Michael Walle wrote: > Am 2023-02-08 15:12, schrieb Greg Kroah-Hartman: > > On Wed, Feb 08, 2023 at 02:36:23PM +0100, Michael Walle wrote: > > > Am 2023-02-08 13:57, schrieb Greg Kroah-Hartman: > > > > When calling debugfs_lookup() the result must have dput() called on it, > > > > otherwise the memory will leak over time. > > > > > > > > Cc: Tudor Ambarus <tudor.ambarus@microchip.com> > > > > Cc: Pratyush Yadav <pratyush@kernel.org> > > > > Cc: Michael Walle <michael@walle.cc> > > > > Cc: Miquel Raynal <miquel.raynal@bootlin.com> > > > > Cc: Richard Weinberger <richard@nod.at> > > > > Cc: Vignesh Raghavendra <vigneshr@ti.com> > > > > Cc: linux-mtd@lists.infradead.org > > > > Cc: stable <stable@kernel.org> > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > --- > > > > v2: fix up to work when module is removed and added, making the fix > > > > much simpler. > > > > > > > > drivers/mtd/spi-nor/debugfs.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/mtd/spi-nor/debugfs.c > > > > b/drivers/mtd/spi-nor/debugfs.c > > > > index ff895f6758ea..af41fbc09a97 100644 > > > > --- a/drivers/mtd/spi-nor/debugfs.c > > > > +++ b/drivers/mtd/spi-nor/debugfs.c > > > > @@ -242,6 +242,7 @@ void spi_nor_debugfs_register(struct spi_nor *nor) > > > > > > > > d = debugfs_create_dir(dev_name(nor->dev), rootdir); > > > > nor->debugfs_root = d; > > > > + dput(rootdir); > > > > > > rootdir might either be the return value of debugfs_lookup() or > > > debugfs_create_dir(). dput() is probably wrong for the latter, > > > right? Also there is an early return, where the dput() is missing, > > > too. > > > > {sigh} > > > > Yeah, this is all wrong, sorry. Let me fix this up again, properly. > > And to do it properly, let's have the module remove the directory if it > > is unloaded, like a good module should :) > > There were some complications. IIRC you'd need to do reference counting, > to determine whether you are the last user of the rootdir. Other subsys > create an empty rootdir in their .init(). But that was hard to do in MTD. > Again memory is hazy.. Therefore, I resorted to create it on the fly if > there isn't already one. > > Maybe you got some better/simpler idea :) Yup, just do it normally like other drivers, I'll send a v3 now. thanks, greg k-h
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c index ff895f6758ea..af41fbc09a97 100644 --- a/drivers/mtd/spi-nor/debugfs.c +++ b/drivers/mtd/spi-nor/debugfs.c @@ -242,6 +242,7 @@ void spi_nor_debugfs_register(struct spi_nor *nor) d = debugfs_create_dir(dev_name(nor->dev), rootdir); nor->debugfs_root = d; + dput(rootdir); debugfs_create_file("params", 0444, d, nor, &spi_nor_params_fops); debugfs_create_file("capabilities", 0444, d, nor,