Hi Andrew and Greg!

On Fri, 2 Sep 2022 09:17:03 -0700 Andrew Morton akpm@linux-foundation.org wrote:

On Fri, 2 Sep 2022 14:56:31 +0200 Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:

...

When calling debugfs_lookup() the result must have dput() called on it, otherwise the memory will leak over time. Fix this up by properly calling dput().

Thank you for finding this bug and posting this patch, Greg!

...

...

Fixes: 75c1c2b53c78b, I assume.

Correct, Andrew.

Fixes: 75c1c2b53c78b ("mm/damon/dbgfs: support multiple contexts") Cc: stable@vger.kernel.org # 5.15.x

...

--- a/mm/damon/dbgfs.c +++ b/mm/damon/dbgfs.c @@ -915,6 +915,7 @@ static int dbgfs_rm_context(char *name) new_ctxs[j++] = dbgfs_ctxs[i]; }

• dput(dir); kfree(dbgfs_dirs); kfree(dbgfs_ctxs);

dput() is also needed if either of the kmalloc_array() calls fail? Maybe something like

Good catch.

--- a/mm/damon/dbgfs.c~a +++ a/mm/damon/dbgfs.c @@ -884,6 +884,7 @@ static int dbgfs_rm_context(char *name) struct dentry *root, *dir, **new_dirs; struct damon_ctx **new_ctxs; int i, j;

• int ret = 0;

if (damon_nr_running_ctxs()) return -EBUSY; @@ -899,14 +900,12 @@ static int dbgfs_rm_context(char *name) new_dirs = kmalloc_array(dbgfs_nr_ctxs - 1, sizeof(*dbgfs_dirs), GFP_KERNEL); if (!new_dirs)

• return -ENOMEM;
  

• goto out_dput;
  

Shouldn't we also do 'ret = -ENOMEM;' before 'godo out_dput'?

new_ctxs = kmalloc_array(dbgfs_nr_ctxs - 1, sizeof(*dbgfs_ctxs), GFP_KERNEL);

• if (!new_ctxs) {

• kfree(new_dirs);
  

• return -ENOMEM;
  

• }

• if (!new_ctxs)

• goto out_new_dirs;
  

ditto.

for (i = 0, j = 0; i < dbgfs_nr_ctxs; i++) { if (dbgfs_dirs[i] == dir) { @@ -925,7 +924,13 @@ static int dbgfs_rm_context(char *name) dbgfs_ctxs = new_ctxs; dbgfs_nr_ctxs--;

• return 0;

• goto out_dput;

+out_new_dirs:

• kfree(new_dirs);

+out_dput:

• dput(dir);
• return ret;

} static ssize_t dbgfs_rm_context_write(struct file *file, _

Thanks, SJ