mm/damon: validate all parameters in the core

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] damon_commit_ctx() silently corrupts the quota charging state of running schemes. --

commit b274d76c061808ff0cd97c6e9433199f816cc1de
Author: SJ Park <[email protected]>

mm/damon/core: safely validate src on damon_commit_ctx()

damon_commit_ctx() does its holistic parameter set validation while applying the new parameter in the set one by one. If it finds a parameter is invalid, because some invalid parameters may already be committed (it is called "commit" but not atomic and irreversable), it stops the running DAMON context.

Quoted 4 lines

mm/damon/core.c +0 -0

index c6fc35be633b3..b523a18a7b971 100644

[ ... ]

mm/damon/core.c int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src) · line 1700

Quoted 13 lines

mm/damon/core.c +9 -0

@@ -1702,6 +1689,52 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
 	return 0;
 }
 
+static struct damon_ctx *damon_new_test_ctx(struct damon_ctx *dst)
+{
+	struct damon_ctx *test_ctx;
+	int err;
+
+	test_ctx = damon_new_ctx();
+	if (!test_ctx)
+		return NULL;
+	err = __damon_commit_ctx(test_ctx, dst);

[Severity: High]
This is a pre-existing issue, but does this unintentionally corrupt the running quota state?

When __damon_commit_ctx() is called with the running context (dst) as the source, it copies the schemes via damon_commit_schemes(), which calls damon_new_scheme() and passes a pointer to the active scheme's quota:

mm/damon/core.c:damon_commit_schemes() {
	...
	new_scheme = damon_new_scheme(&src_scheme->pattern,
				src_scheme->action,
				src_scheme->apply_interval_us,
				&src_scheme->quota, &src_scheme->wmarks,
				NUMA_NO_NODE);
	...
}

damon_new_scheme() then calls damos_quota_init() on that provided quota pointer:

mm/damon/core.c:damon_new_scheme() {
	...
	scheme->quota = *(damos_quota_init(quota));
	...
}

damos_quota_init() zeroes out the fields in-place:

mm/damon/core.c:damos_quota_init() {
	...
	quota->esz = 0;
	quota->total_charged_sz = 0;
	...
}

This mutates the running scheme's quota state, which would make the quota effectively "full" mid-interval and prevent the scheme from applying its actions until the next reset interval.

Since this patch moves this test context logic into the core damon_commit_ctx(), doesn't this expose all API users like damon_reclaim and damon_lru_sort to this state corruption when parameters are committed?

On Thu, 02 Jul 2026 21:21:42 +0000 [email protected] wrote:

Quoted 36 lines

[Severity: High]
This is a pre-existing issue, but does this unintentionally corrupt the running quota state?

When __damon_commit_ctx() is called with the running context (dst) as the source, it copies the schemes via damon_commit_schemes(), which calls damon_new_scheme() and passes a pointer to the active scheme's quota:

mm/damon/core.c:damon_commit_schemes() {
	...
	new_scheme = damon_new_scheme(&src_scheme->pattern,
				src_scheme->action,
				src_scheme->apply_interval_us,
				&src_scheme->quota, &src_scheme->wmarks,
				NUMA_NO_NODE);
	...
}

damon_new_scheme() then calls damos_quota_init() on that provided quota pointer:

mm/damon/core.c:damon_new_scheme() {
	...
	scheme->quota = *(damos_quota_init(quota));
	...
}

damos_quota_init() zeroes out the fields in-place:

mm/damon/core.c:damos_quota_init() {
	...
	quota->esz = 0;
	quota->total_charged_sz = 0;
	...
}

Quoted 3 lines

This mutates the running scheme's quota state, which would make the quota effectively "full" mid-interval and prevent the scheme from applying its actions until the next reset interval.

That doesn't matter because it is only testing purpose scheme, not the schemes of the running context.

Quoted 1 line

Quoted 3 lines

Since this patch moves this test context logic into the core damon_commit_ctx(), doesn't this expose all API users like damon_reclaim and damon_lru_sort to this state corruption when parameters are committed?

So, no problem.

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the early validation of min_region_sz causes persistent corruption of the monitor_region_start and monitor_region_end module parameters on failure. --

commit 982a6b66f4f61086f3bd8de5a2da757081d4fa94
Author: SJ Park <[email protected]>

mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check

This commit removes an explicit power-of-two check on min_region_sz from damon_lru_sort_apply_parameters(), as the same validation is performed later in damon_commit_ctx().

mm/damon/lru_sort.c static int damon_lru_sort_apply_parameters(void) · line 291

Quoted 13 lines

mm/damon/lru_sort.c +0 -5

index 32f41491b726b..2dd0cd0d26273 100644
@@ -284,11 +284,6 @@ static int damon_lru_sort_apply_parameters(void)
 	param_ctx->addr_unit = addr_unit;
 	param_ctx->min_region_sz = max(DAMON_MIN_REGION_SZ / addr_unit, 1);
 
-	if (!is_power_of_2(param_ctx->min_region_sz)) {
-		err = -EINVAL;
-		goto out;
-	}
-

[Severity: High]
Does removing this early validation allow execution to proceed with an invalid addr_unit?

If the monitor_region_start and monitor_region_end parameters are 0 (the default, triggering auto-detection), they are overwritten in-place with system bounds scaled by the invalid addr_unit when this function is called later in damon_lru_sort_apply_parameters():

	err = damon_set_region_system_rams_default(param_target,
					&monitor_region_start,
					&monitor_region_end,
					param_ctx->addr_unit,
					param_ctx->min_region_sz);

When damon_commit_ctx() later rejects the configuration and returns -EINVAL, these corrupted values are not reverted.

If a user then corrects addr_unit and applies again, will the now non-zero parameters bypass auto-detection and get scaled again? This could cause DAMON to monitor an incorrect and massive physical address range, forcing the kdamond thread into a long pfn_valid() loop that consumes excessive CPU.

Is the same state corruption possible in mm/damon/reclaim.c:
damon_reclaim_apply_parameters() due to a similar removal of the early power-of-two check?

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] damon_set_regions() creates duplicate and overlapping regions if input ranges share aligned boundaries, corrupting DAMON's internal state.
- [Medium] damon_set_regions() inconsistently bypasses size validation, silently accepting and re-aligning negative-sized (start > end) ranges instead of rejecting them. --

commit ded29c13e7c58ad791814e55b8139da57477556b
Author: SJ Park <[email protected]>

mm/damon: document region size validation in damon_set_regions()

The kernel doc comment of damon_region clearly specifies every region should have positive size. But it is unclear who should verify it. damon_set_regions() is the recommended DAMON core function for setting regions from the callers, and has the verification.

include/linux/damon.h struct damon_size_range { · line 53

Quoted 11 lines

include/linux/damon.h +2 -1

index 805e089ff4f2f..b3c0ac5bf9b88 100644
@@ -49,7 +49,8 @@ struct damon_size_range {
  * @list:		List head for siblings.
  * @age:		Age of this region.
  *
- * For any use case, @ar should be non-zero positive size.
+ * For any use case, @ar should be non-zero positive size.  damon_set_regions()
+ * does the validation.

[Severity: High]
This is a pre-existing issue, but if callers rely on damon_set_regions() to do the validation, can it create duplicate and overlapping regions when input ranges share aligned boundaries?

If userspace passes multiple valid non-overlapping ranges (e.g., [0, 100] and [100, 200]) that align to the same min_region_sz (e.g., 4096), and the target is empty, the loop in mm/damon/core.c:damon_set_regions() blindly adds aligned ranges without checking for previous alignments:

mm/damon/core.c:damon_set_regions() {
...

			r = damon_new_region(
					ALIGN_DOWN(ranges[i].start,
						min_region_sz),
					ALIGN(ranges[i].end, min_region_sz));
			if (!r)
				return -ENOMEM;
			damon_add_region(r, t);

...
}

When regions exist, the iterator is not reset per input range:

mm/damon/core.c:damon_set_regions() {
...

	r = damon_first_region(t);
	/* Add new regions or resize existing regions to fit in the ranges */
	for (i = 0; i < nr_ranges; i++) {
...
		/* Get the first/last regions intersecting with the range */
		damon_for_each_region_from(r, t) {

...
}

It advances past the first intersection, fails to find the same region for the second range, and creates a duplicate [0, 4096] region, which corrupts DAMON's internal lists.

[Severity: Medium]
This isn't a bug introduced by this patch, but does damon_set_regions() actually fully validate the size?

The validation check in mm/damon/core.c:damon_set_regions() uses aligned boundaries:

mm/damon/core.c:damon_set_regions() {
...

		if (ALIGN_DOWN(ranges[i].start, min_region_sz) >=
				ALIGN(ranges[i].end, min_region_sz))
			return -EINVAL;

...
}

If a user writes a reversed range (e.g., start=4000, end=1000) via sysfs, since 0 >= 4096 evaluates to false, wouldn't the invalid input be silently accepted and transformed into [0, 4096], bypassing the intended size restrictions?

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removing the explicit *start > *end check introduces inconsistent input validation, allowing inverted or zero-size ranges to be silently accepted. --

commit ad7c7a8a7766992d5ad4dbe1c23874e807f45071
Author: SJ Park <[email protected]>

mm/damon/core: remove start, end check in damon_set_region_system_rams()

damon_set_region_system_rams() validates user inputs to avoid creating a negative size region. But DAMON core avoids zero size, too. The check is incomplete. The complete check is done inside damon_set_regions(), which is eventually called from damon_set_region_system_rams_default(). Drop the incomplete and unnecessary check.

mm/damon/core.c int damon_set_region_system_rams_default(struct damon_target *t, · line 3817

Quoted 11 lines

mm/damon/core.c +0 -3

index 179ce6b0a542d..dba53a41a1025 100644
@@ -3812,9 +3812,6 @@ int damon_set_region_system_rams_default(struct damon_target *t,
 {
 	struct damon_addr_range addr_range;
 
-	if (*start > *end)
-		return -EINVAL;
-

[Severity: Medium]
Does removing this check allow invalid inverted or zero-size unaligned bounds to be silently accepted?

The commit message claims that a complete check is done inside damon_set_regions(). However, looking at damon_set_regions(), the validation acts on aligned values:

mm/damon/core.c:damon_set_regions() {
    ...
	for (i = 0; i < nr_ranges; i++) {
		if (ALIGN_DOWN(ranges[i].start, min_region_sz) >=
				ALIGN(ranges[i].end, min_region_sz))
			return -EINVAL;
	}
    ...
}

If unaligned invalid bounds are passed (for example, start=4095 and end=20, or start=20 and end=20), they are rounded to valid blocks (e.g., 0 to 4096) by ALIGN_DOWN and ALIGN.

Because they evaluate to a valid positive range after alignment, they escape the >= check and are silently accepted.

Should this explicit check be kept to ensure unaligned invalid bounds are correctly rejected before alignment?

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the start > end validation allows negative-sized regions to bypass subsequent validation, resulting in overlapping regions and eventually negative-sized regions in the DAMON core, leading to unbounded memory allocation (OOM). --

commit d23d0a5af36c320aac75064c21548cc9de76c4f1
Author: SJ Park <[email protected]>

mm/damon/sysfs: remove region size validation

DAMON_SYSFS validates user inputs for monitoring target regions to disallow negative size regions. DAMON core assumes only positive size regions, though. The validation is incomplete. Fortunately damon_set_regions(), which is eventually used by DAMON_SYSFS, does the complete validation. Remove the incomplete and unnecessary validation.

mm/damon/sysfs.c static int damon_sysfs_set_regions(struct damon_target *t, · line 1992

Quoted 11 lines

mm/damon/sysfs.c +0 -3

index e666dddf1feba..b65651498e0d1 100644
@@ -1987,9 +1987,6 @@ static int damon_sysfs_set_regions(struct damon_target *t,
 		struct damon_sysfs_region *sys_region =
 			sysfs_regions->regions_arr[i];
 
-		if (sys_region->ar.start > sys_region->ar.end)
-			goto out;
-

[Severity: High]
Does removing this validation allow negative-sized regions to bypass the subsequent overlap checks in damon_sysfs_set_regions()?

If we have an inverted region (e.g., start=2000, end=1000) followed by another region (e.g., start=1500, end=3000), the overlap check in damon_sysfs_set_regions() evaluates:

	if (ranges[i - 1].end > ranges[i].start)

Which results in ranges[0].end (1000) > ranges[1].start (1500).

This is false, meaning the inverted region bypasses the overlap check.

Will this inverted region also bypass the alignment-based bounds check in mm/damon/core.c:damon_set_regions()?

mm/damon/core.c:damon_set_regions() {
	...
	for (i = 0; i < nr_ranges; i++) {
		if (ALIGN_DOWN(ranges[i].start, min_region_sz) >=
				ALIGN(ranges[i].end, min_region_sz))
			return -EINVAL;
	}

If the regions fall within the same alignment block (e.g., start=2000 and end=1000 with min_region_sz=4096 align to 0 and 4096), the check evaluates 0 >= 4096, which is false, passing the invalid input to the core.

When the negative-sized region reaches mm/damon/core.c:damon_apply_min_nr_regions(), can this cause an out-of-memory loop?

mm/damon/core.c:damon_apply_min_nr_regions() {
	...
			while (damon_sz_region(r) > max_region_sz) {
				damon_split_region_at(t, r, max_region_sz);
				r = damon_next_region(r);
			}

The inverted region size would underflow to a very large number, which appears to cause this loop to continuously split the region, allocating region structs until the system runs out of memory.