mm/damon/sysfs-schemes: fix use-after-free for [memcg_]path

Sashiko AI review found 1 potential issue(s):
- [High] Breaking from mem_cgroup_iter() without calling mem_cgroup_iter_break() leaks a cgroup reference in damon_sysfs_memcg_path_to_id().

--

commit f19bff910905231adad168cfffc97ed67cb1fde3
Author: SeongJae Park <[email protected]>

mm/damon/sysfs-schemes: protect memcg_path kfree() with damon_sysfs_lock

This commit fixes a use-after-free vulnerability by using damon_sysfs_lock to protect concurrent user-direct reads and writes to the memcg_path sysfs file. The locking prevents readers from accessing the memory path buffer while it is being deallocated by a write operation.

Quoted 4 lines

mm/damon/sysfs-schemes.c +0 -0

index 5186966dafb35..8d32a20531d49 100644

[ ... ]

mm/damon/sysfs-schemes.c static ssize_t memcg_path_store(struct kobject *kobj, · line 566

Quoted 13 lines

mm/damon/sysfs-schemes.c +5 -0

@@ -550,8 +555,13 @@ static ssize_t memcg_path_store(struct kobject *kobj,
 		return -ENOMEM;
 
 	strscpy(path, buf, count + 1);
+	if (!mutex_trylock(&damon_sysfs_lock)) {
+		kfree(path);
+		return -EBUSY;
+	}
 	kfree(filter->memcg_path);
 	filter->memcg_path = path;
+	mutex_unlock(&damon_sysfs_lock);
 	return count;
 }

This isn't a bug introduced by this patch, but I noticed a pre-existing issue in this file.

Does damon_sysfs_memcg_path_to_id() leak a cgroup reference when breaking early from the hierarchy walk?

mm/damon/sysfs-schemes.c:damon_sysfs_memcg_path_to_id() {
	...
	for (memcg = mem_cgroup_iter(NULL, NULL, NULL); memcg;
			memcg = mem_cgroup_iter(NULL, memcg, NULL)) {
		/* skip offlined memcg */
		if (!mem_cgroup_online(memcg))
			continue;
		if (damon_sysfs_memcg_path_eq(memcg, path, memcg_path)) {
			*id = mem_cgroup_id(memcg);
			found = true;
			break;
		}
	}
	...
}

According to the API contract, if a hierarchy walk is aborted prematurely before a round-trip is complete, the caller must use mem_cgroup_iter_break(NULL, memcg) to drop the reference to the last visited cgroup.

Because this is missing, could every successful resolution of a memcg path permanently leak a css reference?

In environments where DAMON configurations are frequently updated (like dynamically profiling containers), this could lead to a buildup of "zombie" cgroups that cannot be destroyed, eventually pinning substantial kernel memory and causing system-wide memory exhaustion.

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Sashiko AI review found 1 potential issue(s):
- [Low] The commit message incorrectly references the memcg_path file instead of the path file. --

commit 9335b00570732d12b29034fc72a20506fd69fbdd
Author: SeongJae Park <[email protected]>

mm/damon/sysfs-schemes: protect path kfree() with damon_sysfs_lock

This commit adds damon_sysfs_lock protection to the path attribute's sysfs show and store functions. This prevents a use-after-free race condition during concurrent user-driven reads and writes.

Quoted 2 lines

damon_sysfs_quot_goal->path can be read and written by users, via DAMON sysfs memcg_path file. It can also be indirectly read, for the

This isn't a bug, but the commit message references the memcg_path sysfs file. Since the patch actually modifies path_show and path_store, which handle the path attribute for quota goals, should this say "via DAMON sysfs path file" instead?

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

Reads of 'path' and 'memcg_path' files in DAMON sysfs interface could race with their writes, results in use-after-free. Fix those.

SeongJae Park (2):
mm/damon/sysfs-schemes: protect memcg_path kfree() with
damon_sysfs_lock
mm/damon/sysfs-schemes: protect path kfree() with damon_sysfs_lock

 mm/damon/sysfs-schemes.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

base-commit: 0d45806f3a75bf53e59475b0e56be324f650ab09

Signature · 1 line

2.47.3