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

[sashiko-bot@kernel.org]

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.
- [Critical] Pre-existing: Use-After-Free and list corruption due to synchronous `kobject_put()` assumption.
- [Medium] Unbalanced `nr_regions` increment on `kobject_init_and_add()` failure causes a permanent state leak.
--

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.

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

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?

>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511191218.98881-1-agarwal.vineet2006@gmail.com?part=1