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

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

[Vineet Agarwal]

damos_sysfs_populate_region_dir() increments
sysfs_regions->nr_regions twice when adding a new region:
once explicitly before kobject_init_and_add(), and once
again through the post-increment used for the kobject name.

As a result, nr_regions no longer matches the actual
number of live regions, and region directory names skip
numbers (1, 3, 5, ...).

Use the already incremented value for naming instead of
incrementing nr_regions a second time.

Fixes: 66178e4ec30a ("mm/damon/sysfs: use damos_walk() for update_schemes_tried_{bytes,regions}")
Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>
---
 mm/damon/sysfs-schemes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 622c3799db87..5d966ac86419 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)) {
 		kobject_put(&region->kobj);
 	}
 }
-- 
2.54.0

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

[SeongJae Park]

On Tue, 12 May 2026 09:41:57 +0530 Vineet Agarwal <agarwal.vineet2006@gmail.com> wrote:

> damos_sysfs_populate_region_dir() increments
> sysfs_regions->nr_regions twice when adding a new region:
> once explicitly before kobject_init_and_add(), and once
> again through the post-increment used for the kobject name.
> 
> As a result, nr_regions no longer matches the actual
> number of live regions, and region directory names skip
> numbers (1, 3, 5, ...).
> 
> Use the already incremented value for naming instead of
> incrementing nr_regions a second time.
> 
> Fixes: 66178e4ec30a ("mm/damon/sysfs: use damos_walk() for update_schemes_tried_{bytes,regions}")

FYI we decided [1] not to Cc stable@ because the bug is not making meaningful
real user issues.  The above Fixes: is only for people who seek more contexts
about from when the bug was introduced.

> Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail.com>

Reviewed-by: SeongJae Park <sj@kernel.org>

> ---

From the next time, please consider adding patch changelog [2] here, with link
to previous revision.  To my understanding, this revision has no change from
the previous revision [1] except dropping the RFC tag.

[1] https://lore.kernel.org/20260511191218.98881-1-agarwal.vineet2006@gmail.com
[2] https://docs.kernel.org/process/submitting-patches.html#commentary


Thanks,
SJ

[...]

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incomplete fix leaves nr_regions counter permanently leaking on the error path.
- [High] List corruption and Double Free due to lockless list manipulation in kobject release.
- [Medium] Region directory naming preserves an unintended 1-based indexing regression.
--

commit 8f7ffd21263a0409418cf4d88e9b7e2d7a90117e
Author: Vineet Agarwal <agarwal.vineet2006@gmail.com>

mm/damon/sysfs-schemes: fix double increment of nr_regions

This commit attempts to fix a double increment of nr_regions when populating
region directories. It removes the post-increment from the
kobject_init_and_add call, relying instead on the earlier increment of
sysfs_regions->nr_regions.

> 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)) {
>  		kobject_put(&region->kobj);
>  	}
>  }

Does this error path leak the counter?

If kobject_init_and_add() fails, kobject_put() triggers
damon_sysfs_scheme_region_release() to delete the region from the list and
free it. However, sysfs_regions->nr_regions has already been incremented
prior to this call and is never decremented on this error path. Because the
cleanup function damon_sysfs_scheme_regions_rm_dirs() only decrements the
counter for items remaining in the list, the counter permanently leaks. After
an error, subsequent directory names will skip numbers and drift indefinitely.

Also, does this preserve an unintended 1-based indexing regression?

Prior to commit 66178e4ec30a, region directories were 0-indexed. By passing
the already-incremented sysfs_regions->nr_regions to kobject_init_and_add(),
the first region directory is named 1. This violates standard DAMON sysfs
array conventions where targets, schemes, and filters all start at 0,
potentially breaking user space tools.

Should the code pass the unincremented sysfs_regions->nr_regions to
kobject_init_and_add() and only increment it on the success path?

Finally, this isn't a bug in this patch, but does the kobject release path
have a lockless list manipulation issue that can cause a double-free?

The function damon_sysfs_scheme_region_release() calls list_del(&region->list)
without holding damon_sysfs_lock. If kobject_release() is delayed (for
example, when CONFIG_DEBUG_KOBJECT_RELEASE is enabled), the region remains
linked in sysfs_regions->regions_list after
damon_sysfs_scheme_regions_rm_dirs() has called kobject_put() on it.

If a subsequent command like update_schemes_tried_regions executes while the
region is awaiting delayed release, damon_sysfs_scheme_regions_rm_dirs() will
traverse the list and call kobject_put() a second time on the same
zero-refcount object. Furthermore, when the delayed release finally executes,
list_del() will run concurrently with other list operations without locks,
resulting in list corruption.

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

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

[SeongJae Park]

On Wed, 13 May 2026 04:21:54 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Incomplete fix leaves nr_regions counter permanently leaking on the error path.
> - [High] List corruption and Double Free due to lockless list manipulation in kobject release.
> - [Medium] Region directory naming preserves an unintended 1-based indexing regression.

Findings are same to those raised for the previous revision of this patch.  My
opinion is also same.  None of these are the fault of this patch.  Only the
list corruption issue seems valid and I will work on it.

Read my previous reply [1] for details.

[1] https://lore.kernel.org/20260513011920.119183-1-sj@kernel.org


Thanks,
SJ

[...]