[PATCH] mm/damon/core: disallow non-power of two min_region_sz on damon_start()

[PATCH] mm/damon/core: disallow non-power of two min_region_sz on damon_start()

[SeongJae Park]

Commit d8f867fa0825 ("mm/damon: add damon_ctx->min_sz_region")
introduced a bug that allows unaligned DAMON region address ranges.
Commit c80f46ac228b ("mm/damon/core: disallow non-power of two
min_region_sz") fixed it, but only for damon_commit_ctx() use case.
Still, DAMON sysfs interface can emit non-power of two min_region_sz via
damon_start().  Fix the path by adding the is_power_of_2() check on
damon_start().

The issue was discovered by sashiko [1].

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

Fixes: d8f867fa0825 ("mm/damon: add damon_ctx->min_sz_region")
Cc: <stable@vger.kernel.org> # 6.18.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 3703f62a876b3..c107d74c77e74 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1368,6 +1368,11 @@ int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)
 	int i;
 	int err = 0;
 
+	for (i = 0; i < nr_ctxs; i++) {
+		if (!is_power_of_2(ctxs[i]->min_region_sz))
+			return -EINVAL;
+	}
+
 	mutex_lock(&damon_lock);
 	if ((exclusive && nr_running_ctxs) ||
 			(!exclusive && running_exclusive_ctxs)) {

base-commit: 872b6168e577ea326324255fa8b6716a89124680
-- 
2.47.3

Re: (sashiko review) [PATCH] mm/damon/core: disallow non-power of two min_region_sz on damon_start()

[SeongJae Park]

TL; DR: sashiko found no blocker of this patch.

Forwarding full sashiko review in a reply format with my 
inline comments below, for sharing details of my view and
doing followup discussions via mails if needed.

> # review url: https://sashiko.dev/#/patchset/20260411213638.77768-1-sj@kernel.org
> # start of sashiko.dev inline review
> commit a125056db470cab021e84ad481a0f51ddfc426c2
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: disallow non-power of two min_region_sz on damon_start()
> 
> This commit fixes an issue that allows unaligned DAMON region address ranges.
> It adds a check for power-of-two min_region_sz directly in damon_start()
> to catch invalid configurations from the sysfs interface.
> 
> Link: https://lore.kernel.org/20260403155530.64647-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3703f62a876b3..c107d74c77e74 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -1368,6 +1368,11 @@ int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)
> >  	int i;
> >  	int err = 0;
> >  
> > +	for (i = 0; i < nr_ctxs; i++) {
> > +		if (!is_power_of_2(ctxs[i]->min_region_sz))
> > +			return -EINVAL;
> > +	}
> > +
> 
> This isn't a regression introduced by this commit, but modifying
> damon_start() brings up a question about partial failure handling when
> starting multiple contexts.
> 
> If __damon_start() succeeds for the first context but fails for a subsequent
> one (for example if kthread_run() fails), the initialization loop breaks and
> returns an error:
> 
> 	for (i = 0; i < nr_ctxs; i++) {
> 		err = __damon_start(ctxs[i]);
> 		if (err)
> 			break;
> 		nr_running_ctxs++;
> 	}
> 
> Does this leave the previously successful kdamond threads running without
> stopping them?
> 
> Because nr_running_ctxs is incremented for the successful starts and
> running_exclusive_ctxs is set to true, it appears the internal state
> would prevent starting new contexts.
> 
> Would this result in all future calls to damon_start() returning -EBUSY?

Yes, this is not ideal.  But as Sashiko also says it is no blocker of this
patch.  I will revisit this later.

> 
> 
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260411213638.77768-1-sj@kernel.org


Thanks,
SJ

# hkml [1] generated a draft of this mail.  You can regenerate
# this using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260411213638.77768-1-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail