Re: [RFC PATCH] mm/damon/sysfs-schemes: fix double increment of nr_regions

[SeongJae Park <sj@kernel.org>]

On Wed, 13 May 2026 00:08:38 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Critical] The patch fails to restore the documented 0-indexed ABI, leaving region directory names 1-indexed.

Not this patch's fault.  No real issue.

> - [Critical] Pre-existing: Use-After-Free and list corruption due to synchronous `kobject_put()` assumption.

I will work on this.

> - [Medium] Unbalanced `nr_regions` increment on `kobject_init_and_add()` failure causes a permanent state leak.

Not this patch's fault.  No real issue.

> --
> 
> commit 66d182db7d03e02c982ddd51e0ba311b2642559b
> Author: Vineet Agarwal <agarwal.vineet2006@gmail.com>
> 
> mm/damon/sysfs-schemes: fix double increment of nr_regions
> 
> This commit changes damos_sysfs_populate_region_dir() to use the already
> incremented value of nr_regions for naming new region directories,
> preventing the skipping of directory numbers.
> 
> > diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> > index 622c3799db87b..5d966ac864193 100644
> > --- a/mm/damon/sysfs-schemes.c
> > +++ b/mm/damon/sysfs-schemes.c
> > @@ -2998,7 +2998,7 @@ void damos_sysfs_populate_region_dir(struct damon_sysfs_schemes *sysfs_schemes,
> >  	if (kobject_init_and_add(&region->kobj,
> >  				&damon_sysfs_scheme_region_ktype,
> >  				&sysfs_regions->kobj, "%d",
> > -				sysfs_regions->nr_regions++)) {
> > +				sysfs_regions->nr_regions)) {
> 
> Does this change the ABI from 0-indexed to 1-indexed? Since nr_regions is
> incremented just before kobject_init_and_add(), the first directory will be
> formatted with the value "1". The kernel documentation indicates these
> directories should start from 0.

Not a new behavior.  And it should be fine.  Users could reasonably deal with
it.  Refer to my first reply to the patch for more details.

> 
> >  		kobject_put(&region->kobj);
> 
> If kobject_init_and_add() fails here, we call kobject_put(&region->kobj) but
> do not decrement sysfs_regions->nr_regions. Will this permanently inflate the
> counter and cause subsequent directory names to skip numbers, since
> damon_sysfs_scheme_regions_rm_dirs() decrements the counter based on the
> remaining successfully added elements in the list?

Yes, and it is an old behavior, so nothing to blame for this patch.  Also same
to the 1-index, users should be able to reasonably handle it.

> 
> There might also be a pre-existing issue with the region list lifecycle. If
> damon_sysfs_scheme_regions_rm_dirs() iterates regions_list and calls
> kobject_put() on each region's kobj, does it assume the kobject release
> function will synchronously remove the region from the list?
> 
> If the release is delayed, for instance by external sysfs references or
> CONFIG_DEBUG_KOBJECT_RELEASE, the object could remain on regions_list. If
> damos_sysfs_populate_region_dir() then concurrently adds to regions_list
> with list_add_tail(), could the delayed release race and cause list
> corruption?
> 
> Could a subsequent call to damon_sysfs_scheme_regions_rm_dirs() traverse
> freed list nodes and call kobject_put() on zero-refcount objects if the
> release hasn't completed?

Good finding, I will work on this.


Thanks,
SJ

[...]