Skip to contentSkip to conversation

DAMON fixes

PATCH Applied SeongJae Park 2 participants 4 messages Nov 10, 2021 – Nov 16, 2021

Subsystems (2)
Memory management DAMON
lore
Share & tools
lore

SeongJae Park

author maintainer
#0 lore raw

cover DAMON fixes

This patchset fixes bugs in DAMON. Those cannot cleanly applied on v5.15.y. I will back-port these on v5.15.y and post later once these are merged in the mainline.

SeongJae Park (2):
mm/damon/dbgfs: Use '__GFP_NOWARN' for user-specified size buffer
allocation
mm/damon/dbgfs: Fix missed use of damon_dbgfs_lock

 mm/damon/dbgfs.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)
Signature · 1 line
2.17.1
author maintainer
#1 lore raw

PATCH 1/2 mm/damon/dbgfs: Use '__GFP_NOWARN' for user-specified size buffer allocation

DAMON users can trigger below warning in '__alloc_pages()' by invoking write() to some DAMON debugfs files with arbitrarily high count argument, because DAMON debugfs interface allocates some buffers based on the user-specified 'count'.

        if (unlikely(order >= MAX_ORDER)) {
                WARN_ON_ONCE(!(gfp & __GFP_NOWARN));
                return NULL;
        }

Because the DAMON debugfs interface code checks failure of the 'kmalloc()', this commit simply suppresses the warnings by adding '__GFP_NOWARN' flag.

Fixes 4bc05954d007 ("mm/damon: implement a debugfs-based user space interface")
Signed-off-by SeongJae Park <[email protected]>
---
 mm/damon/dbgfs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
mm/damon/dbgfs.c +4 -4
index eccc14b34901..8ce1311ac533 100644
@@ -32,7 +32,7 @@ static char *user_input_str(const char __user *buf, size_t count, loff_t *ppos)
 	if (*ppos)
 		return ERR_PTR(-EINVAL);
 
-	kbuf = kmalloc(count + 1, GFP_KERNEL);
+	kbuf = kmalloc(count + 1, GFP_KERNEL | __GFP_NOWARN);
 	if (!kbuf)
 		return ERR_PTR(-ENOMEM);
 

@@ -133,7 +133,7 @@ static ssize_t dbgfs_schemes_read(struct file *file, char __user *buf,
 	char *kbuf;
 	ssize_t len;
 
-	kbuf = kmalloc(count, GFP_KERNEL);
+	kbuf = kmalloc(count, GFP_KERNEL | __GFP_NOWARN);
 	if (!kbuf)
 		return -ENOMEM;
 

@@ -452,7 +452,7 @@ static ssize_t dbgfs_init_regions_read(struct file *file, char __user *buf,
 	char *kbuf;
 	ssize_t len;
 
-	kbuf = kmalloc(count, GFP_KERNEL);
+	kbuf = kmalloc(count, GFP_KERNEL | __GFP_NOWARN);
 	if (!kbuf)
 		return -ENOMEM;
 

@@ -578,7 +578,7 @@ static ssize_t dbgfs_kdamond_pid_read(struct file *file,
 	char *kbuf;
 	ssize_t len;
 
-	kbuf = kmalloc(count, GFP_KERNEL);
+	kbuf = kmalloc(count, GFP_KERNEL | __GFP_NOWARN);
 	if (!kbuf)
 		return -ENOMEM;
Signature · 1 line
2.17.1
author maintainer
#2 lore raw

PATCH 2/2 mm/damon/dbgfs: Fix missed use of damon_dbgfs_lock

In reply to SeongJae Park

DAMON debugfs is supposed to protect dbgfs_ctxs, dbgfs_nr_ctxs, and dbgfs_dirs using damon_dbgfs_lock. However, some of the code is accessing the variables without the protection. This commit fixes it by protecting all such accesses.

Fixes 75c1c2b53c78 ("mm/damon/dbgfs: support multiple contexts")
Signed-off-by SeongJae Park <[email protected]>
---
 mm/damon/dbgfs.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
mm/damon/dbgfs.c +9 -3
index 8ce1311ac533..9b520bb4a3e7 100644
@@ -877,12 +877,14 @@ static ssize_t dbgfs_monitor_on_write(struct file *file,
 		return -EINVAL;
 	}
 
+	mutex_lock(&damon_dbgfs_lock);
 	if (!strncmp(kbuf, "on", count)) {
 		int i;
 
 		for (i = 0; i < dbgfs_nr_ctxs; i++) {
 			if (damon_targets_empty(dbgfs_ctxs[i])) {
 				kfree(kbuf);
+				mutex_unlock(&damon_dbgfs_lock);
 				return -EINVAL;
 			}
 		}

@@ -892,6 +894,7 @@ static ssize_t dbgfs_monitor_on_write(struct file *file,
 	} else {
 		ret = -EINVAL;
 	}
+	mutex_unlock(&damon_dbgfs_lock);
 
 	if (!ret)
 		ret = count;

@@ -944,15 +947,16 @@ static int __init __damon_dbgfs_init(void)
 
 static int __init damon_dbgfs_init(void)
 {
-	int rc;
+	int rc = -ENOMEM;
 
+	mutex_lock(&damon_dbgfs_lock);
 	dbgfs_ctxs = kmalloc(sizeof(*dbgfs_ctxs), GFP_KERNEL);
 	if (!dbgfs_ctxs)
-		return -ENOMEM;
+		goto out;
 	dbgfs_ctxs[0] = dbgfs_new_ctx();
 	if (!dbgfs_ctxs[0]) {
 		kfree(dbgfs_ctxs);
-		return -ENOMEM;
+		goto out;
 	}
 	dbgfs_nr_ctxs = 1;
 

@@ -963,6 +967,8 @@ static int __init damon_dbgfs_init(void)
 		pr_err("%s: dbgfs init failed\n", __func__);
 	}
 
+out:
+	mutex_unlock(&damon_dbgfs_lock);
 	return rc;
 }
Signature · 1 line
2.17.1
maintainer
#3 lore raw
In reply to SeongJae Park
Quoted 3 lines · On Wed, 10 Nov 2021 14:57:56 +0000 SeongJae Park <[email protected]> wrote:

This patchset fixes bugs in DAMON. Those cannot cleanly applied on v5.15.y. I will back-port these on v5.15.y and post later once these are merged in the mainline.

Thanks. I added cc:stable to these even though they won't apply. I think that fits Greg's processes better.