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

[SeongJae Park <sj@kernel.org>]

On Wed, 22 Apr 2026 20:30:16 +0000 sashiko-bot@kernel.org wrote:

> 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()`.

Nice finding, but not this patch's fault.  I will separately work on this.

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

Nice finding.  As Sashiko also mentions, not this patch's fault, though.  I
will separately work on this.

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


Thanks,
SJ