2023-02-02 14:20:33

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH] mm/slub: fix memory leak with using debugfs_lookup()

When calling debugfs_lookup() the result must have dput() called on it,
otherwise the memory will leak over time. To make things simpler, just
call debugfs_lookup_and_remove() instead which handles all of the logic
at once.

Cc: Christoph Lameter <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Joonsoo Kim <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Roman Gushchin <[email protected]>
Cc: Hyeonggon Yoo <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
mm/slub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slub.c b/mm/slub.c
index 13459c69095a..4880e461fcc5 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -6449,7 +6449,7 @@ static void debugfs_slab_add(struct kmem_cache *s)

void debugfs_slab_release(struct kmem_cache *s)
{
- debugfs_remove_recursive(debugfs_lookup(s->name, slab_debugfs_root));
+ debugfs_lookup_and_remove(s->name, slab_debugfs_root);
}

static int __init slab_debugfs_init(void)
--
2.39.1



2023-02-03 14:27:29

by Hyeonggon Yoo

[permalink] [raw]
Subject: Re: [PATCH] mm/slub: fix memory leak with using debugfs_lookup()

On Thu, Feb 02, 2023 at 03:20:22PM +0100, Greg Kroah-Hartman wrote:
> When calling debugfs_lookup() the result must have dput() called on it,
> otherwise the memory will leak over time. To make things simpler, just
> call debugfs_lookup_and_remove() instead which handles all of the logic
> at once.
>
> Cc: Christoph Lameter <[email protected]>
> Cc: Pekka Enberg <[email protected]>
> Cc: David Rientjes <[email protected]>
> Cc: Joonsoo Kim <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Roman Gushchin <[email protected]>
> Cc: Hyeonggon Yoo <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> mm/slub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 13459c69095a..4880e461fcc5 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -6449,7 +6449,7 @@ static void debugfs_slab_add(struct kmem_cache *s)
>
> void debugfs_slab_release(struct kmem_cache *s)
> {
> - debugfs_remove_recursive(debugfs_lookup(s->name, slab_debugfs_root));
> + debugfs_lookup_and_remove(s->name, slab_debugfs_root);
> }

Looks good to me.
Reviewed-by: Hyeonggon Yoo <[email protected]>

Thank you for fixing this.

> static int __init slab_debugfs_init(void)
> --
> 2.39.1
>

2023-02-03 17:48:47

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH] mm/slub: fix memory leak with using debugfs_lookup()

On Thu, Feb 02, 2023 at 03:20:22PM +0100, Greg Kroah-Hartman wrote:
> When calling debugfs_lookup() the result must have dput() called on it,
> otherwise the memory will leak over time. To make things simpler, just
> call debugfs_lookup_and_remove() instead which handles all of the logic
> at once.
>
> Cc: Christoph Lameter <[email protected]>
> Cc: Pekka Enberg <[email protected]>
> Cc: David Rientjes <[email protected]>
> Cc: Joonsoo Kim <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Roman Gushchin <[email protected]>
> Cc: Hyeonggon Yoo <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>

Good catch!

Acked-by: Roman Gushchin <[email protected]>

Thanks!

2023-02-05 22:27:43

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH] mm/slub: fix memory leak with using debugfs_lookup()

On Thu, 2 Feb 2023, Greg Kroah-Hartman wrote:

> When calling debugfs_lookup() the result must have dput() called on it,
> otherwise the memory will leak over time. To make things simpler, just
> call debugfs_lookup_and_remove() instead which handles all of the logic
> at once.
>
> Cc: Christoph Lameter <[email protected]>
> Cc: Pekka Enberg <[email protected]>
> Cc: David Rientjes <[email protected]>
> Cc: Joonsoo Kim <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Roman Gushchin <[email protected]>
> Cc: Hyeonggon Yoo <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>

Acked-by: David Rientjes <[email protected]>

2023-02-06 15:39:04

by Vlastimil Babka

[permalink] [raw]
Subject: Re: [PATCH] mm/slub: fix memory leak with using debugfs_lookup()

On 2/2/23 15:20, Greg Kroah-Hartman wrote:
> When calling debugfs_lookup() the result must have dput() called on it,
> otherwise the memory will leak over time. To make things simpler, just
> call debugfs_lookup_and_remove() instead which handles all of the logic
> at once.
>
> Cc: Christoph Lameter <[email protected]>
> Cc: Pekka Enberg <[email protected]>
> Cc: David Rientjes <[email protected]>
> Cc: Joonsoo Kim <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Vlastimil Babka <[email protected]>
> Cc: Roman Gushchin <[email protected]>
> Cc: Hyeonggon Yoo <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>

Thanks, added to slab/for-6.3/fixes

> ---
> mm/slub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 13459c69095a..4880e461fcc5 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -6449,7 +6449,7 @@ static void debugfs_slab_add(struct kmem_cache *s)
>
> void debugfs_slab_release(struct kmem_cache *s)
> {
> - debugfs_remove_recursive(debugfs_lookup(s->name, slab_debugfs_root));
> + debugfs_lookup_and_remove(s->name, slab_debugfs_root);
> }
>
> static int __init slab_debugfs_init(void)