Re: [RFC PATCH 1/2] mm/damon/sysfs-schemes: protect memcg_path kfree() with damon_sysfs_lock

[sashiko-bot@kernel.org]

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 <sj@kernel.org>

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.

> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index 5186966dafb35..8d32a20531d49 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c

[ ... ]

> @@ -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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260422143503.71357-1-sj@kernel.org?part=1