All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] mtd: spi-nor: fix memory leak when using debugfs_lookup()
@ 2023-02-08 14:53 ` Greg Kroah-Hartman
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2023-02-08 14:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, Tudor Ambarus, Pratyush Yadav, Michael Walle,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, linux-mtd

When calling debugfs_lookup() the result must have dput() called on it,
otherwise the memory will leak over time.  To solve this, remove the
lookup and create the directory on the first device found, and then
remove it when the module is unloaded.

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
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v3: create the directory the first time it is used, and then remove it
    if it is present when the module is unloaded.  More complex than v2,
    but correct.
v2: fix up to work when module is removed and added, making the fix
    much simpler.

 drivers/mtd/spi-nor/core.c    | 14 +++++++++++++-
 drivers/mtd/spi-nor/core.h    |  2 ++
 drivers/mtd/spi-nor/debugfs.c | 12 +++++++++---
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index d67c926bca8b..adf85ce7fdb3 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3335,7 +3335,19 @@ static struct spi_mem_driver spi_nor_driver = {
 	.remove = spi_nor_remove,
 	.shutdown = spi_nor_shutdown,
 };
-module_spi_mem_driver(spi_nor_driver);
+
+static int __init spi_nor_module_init(void)
+{
+	return spi_mem_driver_register(&spi_nor_driver);
+}
+module_init(spi_nor_module_init);
+
+static void __exit spi_nor_module_exit(void)
+{
+	spi_nor_debugfs_shutdown();
+	spi_mem_driver_unregister(&spi_nor_driver);
+}
+module_exit(spi_nor_module_exit);
 
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Huang Shijie <shijie8@gmail.com>");
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index f03b55cf7e6f..e62cd9964456 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -713,8 +713,10 @@ static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
 
 #ifdef CONFIG_DEBUG_FS
 void spi_nor_debugfs_register(struct spi_nor *nor);
+void spi_nor_debugfs_shutdown(void);
 #else
 static inline void spi_nor_debugfs_register(struct spi_nor *nor) {}
+static inline void spi_nor_debugfs_shutdown(void) {}
 #endif
 
 #endif /* __LINUX_MTD_SPI_NOR_INTERNAL_H */
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index ff895f6758ea..cd9c2dc07509 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -226,13 +226,13 @@ static void spi_nor_debugfs_unregister(void *data)
 	nor->debugfs_root = NULL;
 }
 
+static struct dentry *rootdir;
+
 void spi_nor_debugfs_register(struct spi_nor *nor)
 {
-	struct dentry *rootdir, *d;
+	struct dentry *d;
 	int ret;
 
-	/* Create rootdir once. Will never be deleted again. */
-	rootdir = debugfs_lookup(SPI_NOR_DEBUGFS_ROOT, NULL);
 	if (!rootdir)
 		rootdir = debugfs_create_dir(SPI_NOR_DEBUGFS_ROOT, NULL);
 
@@ -247,3 +247,9 @@ void spi_nor_debugfs_register(struct spi_nor *nor)
 	debugfs_create_file("capabilities", 0444, d, nor,
 			    &spi_nor_capabilities_fops);
 }
+
+void spi_nor_debugfs_shutdown(void)
+{
+	if (rootdir)
+		debugfs_remove(rootdir);
+}
-- 
2.39.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3] mtd: spi-nor: fix memory leak when using debugfs_lookup()
@ 2023-02-08 14:53 ` Greg Kroah-Hartman
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2023-02-08 14:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, Tudor Ambarus, Pratyush Yadav, Michael Walle,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, linux-mtd

When calling debugfs_lookup() the result must have dput() called on it,
otherwise the memory will leak over time.  To solve this, remove the
lookup and create the directory on the first device found, and then
remove it when the module is unloaded.

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
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v3: create the directory the first time it is used, and then remove it
    if it is present when the module is unloaded.  More complex than v2,
    but correct.
v2: fix up to work when module is removed and added, making the fix
    much simpler.

 drivers/mtd/spi-nor/core.c    | 14 +++++++++++++-
 drivers/mtd/spi-nor/core.h    |  2 ++
 drivers/mtd/spi-nor/debugfs.c | 12 +++++++++---
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index d67c926bca8b..adf85ce7fdb3 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3335,7 +3335,19 @@ static struct spi_mem_driver spi_nor_driver = {
 	.remove = spi_nor_remove,
 	.shutdown = spi_nor_shutdown,
 };
-module_spi_mem_driver(spi_nor_driver);
+
+static int __init spi_nor_module_init(void)
+{
+	return spi_mem_driver_register(&spi_nor_driver);
+}
+module_init(spi_nor_module_init);
+
+static void __exit spi_nor_module_exit(void)
+{
+	spi_nor_debugfs_shutdown();
+	spi_mem_driver_unregister(&spi_nor_driver);
+}
+module_exit(spi_nor_module_exit);
 
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Huang Shijie <shijie8@gmail.com>");
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index f03b55cf7e6f..e62cd9964456 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -713,8 +713,10 @@ static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
 
 #ifdef CONFIG_DEBUG_FS
 void spi_nor_debugfs_register(struct spi_nor *nor);
+void spi_nor_debugfs_shutdown(void);
 #else
 static inline void spi_nor_debugfs_register(struct spi_nor *nor) {}
+static inline void spi_nor_debugfs_shutdown(void) {}
 #endif
 
 #endif /* __LINUX_MTD_SPI_NOR_INTERNAL_H */
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index ff895f6758ea..cd9c2dc07509 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -226,13 +226,13 @@ static void spi_nor_debugfs_unregister(void *data)
 	nor->debugfs_root = NULL;
 }
 
+static struct dentry *rootdir;
+
 void spi_nor_debugfs_register(struct spi_nor *nor)
 {
-	struct dentry *rootdir, *d;
+	struct dentry *d;
 	int ret;
 
-	/* Create rootdir once. Will never be deleted again. */
-	rootdir = debugfs_lookup(SPI_NOR_DEBUGFS_ROOT, NULL);
 	if (!rootdir)
 		rootdir = debugfs_create_dir(SPI_NOR_DEBUGFS_ROOT, NULL);
 
@@ -247,3 +247,9 @@ void spi_nor_debugfs_register(struct spi_nor *nor)
 	debugfs_create_file("capabilities", 0444, d, nor,
 			    &spi_nor_capabilities_fops);
 }
+
+void spi_nor_debugfs_shutdown(void)
+{
+	if (rootdir)
+		debugfs_remove(rootdir);
+}
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] mtd: spi-nor: fix memory leak when using debugfs_lookup()
  2023-02-08 14:53 ` Greg Kroah-Hartman
@ 2023-02-08 15:28   ` Michael Walle
  -1 siblings, 0 replies; 6+ messages in thread
From: Michael Walle @ 2023-02-08 15:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-mtd

> +static void __exit spi_nor_module_exit(void)
> +{
> +	spi_nor_debugfs_shutdown();
> +	spi_mem_driver_unregister(&spi_nor_driver);

Hm, does the order matter here? I didn't test it, but this will
lead to:

rootdir = debugfs_create_dir("spi-nor", NULL);
nor->debugfs_root = debugfs_create_dir("spi0", rootdir);
debugfs_remove(rootdir);
debugfs_remove(nor->debugfs_root);

If that's ok, then this looks good.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] mtd: spi-nor: fix memory leak when using debugfs_lookup()
@ 2023-02-08 15:28   ` Michael Walle
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Walle @ 2023-02-08 15:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-mtd

> +static void __exit spi_nor_module_exit(void)
> +{
> +	spi_nor_debugfs_shutdown();
> +	spi_mem_driver_unregister(&spi_nor_driver);

Hm, does the order matter here? I didn't test it, but this will
lead to:

rootdir = debugfs_create_dir("spi-nor", NULL);
nor->debugfs_root = debugfs_create_dir("spi0", rootdir);
debugfs_remove(rootdir);
debugfs_remove(nor->debugfs_root);

If that's ok, then this looks good.

-michael

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] mtd: spi-nor: fix memory leak when using debugfs_lookup()
  2023-02-08 15:28   ` Michael Walle
@ 2023-02-08 15:54     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2023-02-08 15:54 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-kernel, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-mtd

On Wed, Feb 08, 2023 at 04:28:45PM +0100, Michael Walle wrote:
> > +static void __exit spi_nor_module_exit(void)
> > +{
> > +	spi_nor_debugfs_shutdown();
> > +	spi_mem_driver_unregister(&spi_nor_driver);
> 
> Hm, does the order matter here? I didn't test it, but this will
> lead to:
> 
> rootdir = debugfs_create_dir("spi-nor", NULL);
> nor->debugfs_root = debugfs_create_dir("spi0", rootdir);
> debugfs_remove(rootdir);
> debugfs_remove(nor->debugfs_root);
> 
> If that's ok, then this looks good.

It's ok as the last call to debugfs_remove() will just fail as that
dentry is long gone.  Not the nicest, so I'll reverse them and send out
a v4 now.

thanks,

greg k-h

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] mtd: spi-nor: fix memory leak when using debugfs_lookup()
@ 2023-02-08 15:54     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2023-02-08 15:54 UTC (permalink / raw)
  To: Michael Walle
  Cc: linux-kernel, Tudor Ambarus, Pratyush Yadav, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, linux-mtd

On Wed, Feb 08, 2023 at 04:28:45PM +0100, Michael Walle wrote:
> > +static void __exit spi_nor_module_exit(void)
> > +{
> > +	spi_nor_debugfs_shutdown();
> > +	spi_mem_driver_unregister(&spi_nor_driver);
> 
> Hm, does the order matter here? I didn't test it, but this will
> lead to:
> 
> rootdir = debugfs_create_dir("spi-nor", NULL);
> nor->debugfs_root = debugfs_create_dir("spi0", rootdir);
> debugfs_remove(rootdir);
> debugfs_remove(nor->debugfs_root);
> 
> If that's ok, then this looks good.

It's ok as the last call to debugfs_remove() will just fail as that
dentry is long gone.  Not the nicest, so I'll reverse them and send out
a v4 now.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-02-08 15:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-08 14:53 [PATCH v3] mtd: spi-nor: fix memory leak when using debugfs_lookup() Greg Kroah-Hartman
2023-02-08 14:53 ` Greg Kroah-Hartman
2023-02-08 15:28 ` Michael Walle
2023-02-08 15:28   ` Michael Walle
2023-02-08 15:54   ` Greg Kroah-Hartman
2023-02-08 15:54     ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.