mm/damon: validate all parameters in the core

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Validation step incorrectly tests the existing context (dst) instead of the new parameters (src), completely neutralizing the patch's safety guarantees. --

commit 4ac81fdba8a8c8576fad127fdd4347015f4dac51
Author: SJ Park <[email protected]>

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

This commit updates damon_commit_ctx() to validate parameters safely by applying them to a test context before applying them to the running context. If the test context validation succeeds, the real update proceeds.

Quoted 4 lines

mm/damon/core.c +0 -0

index 5febe58a07028..5ca176eb27e1d 100644

[ ... ]

Quoted 1 line

mm/damon/core.c +0 -0

@@ -1635,6 +1622,52 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)

[ ... ]

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

Quoted 9 lines

mm/damon/core.c +9 -0

+int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
+{
+	struct damon_ctx *test_ctx;
+	int err;
+
+	test_ctx = damon_new_test_ctx(dst);
+	if (!test_ctx)
+		return -ENOMEM;
+	err = __damon_commit_ctx(test_ctx, dst);

[Severity: High]
Does this code incorrectly validate the existing context instead of the new parameters?

test_ctx is initialized as a clone of dst via damon_new_test_ctx(dst). If __damon_commit_ctx() applies dst to test_ctx again, it avoids testing the new parameters in src entirely.

If src contains invalid parameters (like a non-power-of-two min_region_sz), it will bypass this check. When the unvalidated src is subsequently applied to the running context:

    err = __damon_commit_ctx(dst, src);

it can fail partway through. This leaves dst->maybe_corrupted = true and causes the running kdamond monitoring thread to terminate unexpectedly.

Should the validation step be checking src instead?

    err = __damon_commit_ctx(test_ctx, src);

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] damon_commit_ctx() incorrectly validates dst against test_ctx instead of src, bypassing the safety mechanism.
- [Medium] Reusing a single test_ctx across multiple loop iterations in damon_start() leaks state between contexts, leading to non-deterministic validation outcomes. --

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

mm/damon/core: do parameter testing commit on damon_start()

damon_start() and damon_commit_ctx() are the two main API functions for setting DAMON parameters. This commit embeds the parameter check already present in damon_commit_ctx() into damon_start(), allowing callers to safely use damon_start() without manually validating parameters first.

The commit message mentions that damon_commit_ctx() does holistic parameters testing. Looking at damon_commit_ctx() in the same file:

mm/damon/core.c:damon_commit_ctx() {
	...
	test_ctx = damon_new_test_ctx(dst);
	if (!test_ctx)
		return -ENOMEM;
	err = __damon_commit_ctx(test_ctx, dst);
	if (err)
		goto out;
	err = __damon_commit_ctx(dst, src);
	...
}

[Severity: High]
Does this bypass the safety validation mechanism in damon_commit_ctx()?

The validation incorrectly tests the active context dst against test_ctx instead of testing the incoming parameters src.

If invalid parameters are supplied or if a mid-commit allocation fails, the incoming parameters src are directly applied to the active context dst without any prior dry-run validation. This leaves the active context dst in a corrupted state (maybe_corrupted = true), potentially leading to monitoring thread crashes or unpredictable kernel behavior.

Quoted 5 lines

mm/damon/core.c +0 -0

index 5ca176eb27e1d..ace202f47dd7c 100644

[ ... ]

Quoted 6 lines

mm/damon/core.c +1 -0

@@ -1785,13 +1787,22 @@ static int __damon_start(struct damon_ctx *ctx)
  */
 int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)
 {
+	struct damon_ctx *test_ctx;
 	int i;

mm/damon/core.c static int __damon_start(struct damon_ctx *ctx) · line 1799

Quoted 10 lines

mm/damon/core.c +5 -2

 	int err = 0;
 
+	test_ctx = damon_new_ctx();
+	if (!test_ctx)
+		return -ENOMEM;
+
 	for (i = 0; i < nr_ctxs; i++) {
-		if (!is_power_of_2(ctxs[i]->min_region_sz))
-			return -EINVAL;
+		err = __damon_commit_ctx(test_ctx, ctxs[i]);

[Severity: Medium]
Does reusing a single test_ctx across multiple loop iterations leak state between contexts?

If a caller invokes damon_start() with multiple contexts (nr_ctxs > 1), test_ctx is mutated in the first iteration. In the second iteration, __damon_commit_ctx() runs on the same, un-cleared test_ctx.

For example, if ctxs[1] has an obsolete target:

mm/damon/core.c:damon_commit_targets() {
	...
		/* target to remove has no matching dst */
		if (src_target->obsolete) {
			err = -EINVAL;
			goto out;
		}
	...
}

It evaluates against the residual targets of ctxs[0]. If ctxs[0] had enough targets, the obsolete target incorrectly matches and passes validation. If ctxs[0] had no targets, it fails with -EINVAL, producing non-deterministic validation results.

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the explicit test_ctx parameter validation in damon_sysfs_commit_input exposes an underlying flaw in damon_commit_ctx, allowing invalid sysfs inputs to corrupt the running context and terminate the background kdamond thread. --

Note: Due to system limitations regarding exact code recitation, this report uses a free-form summary of the issue rather than inline patch quoting.

commit bec0828fbe945683bcef8cfdb56247414f2510d8
Author: SJ Park <[email protected]>
Subject: mm/damon/sysfs: remove duplicated commit input validity check

This commit removes the explicit test_ctx validation inside damon_sysfs_commit_input(), assuming the same validation logic is safely embedded inside damon_commit_ctx().

[Severity: High]
Does removing the sysfs-level test_ctx wrapper expose the running context to corruption from invalid user inputs?

In mm/damon/core.c:damon_commit_ctx(), it appears the underlying validation logic tests 'dst' against the temporary 'test_ctx' instead of testing the new 'src' parameters:

damon_commit_ctx()

   err = __damon_commit_ctx(test_ctx, dst);
   if (err)
       goto out;
   err = __damon_commit_ctx(dst, src);

Because damon_sysfs_commit_input() now relies entirely on damon_commit_ctx() for validation, what happens if invalid configuration parameters (such as a min_region_sz that is not a power of 2) are passed via the sysfs interface?

It looks like __damon_commit_ctx(dst, src) will fail halfway through, returning -EINVAL but leaving the running context with maybe_corrupted = true. Can this cause the background kdamond_fn thread to silently abort without recovering, effectively resulting in a denial of service for the monitoring session?

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the early is_power_of_2() check allows invalid min_region_sz inputs to reach damon_commit_ctx(), which exposes a bug introduced earlier in this series that corrupts the live context and silently terminates the DAMON background thread. --

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

mm/damon/reclaim: remove duplicated min_region_sz power of 2 check

DAMON_RECLAIM validates the user input for min_region_sz. The same validation is done inside damon_start() and damon_commit_ctx(). Remove the duplicate.

mm/damon/reclaim.c static int damon_reclaim_apply_parameters(void) · line 214

Quoted 12 lines

mm/damon/reclaim.c +0 -4

index 11b70d0a9a6f0..6469b25cc34f9 100644
@@ -208,11 +208,6 @@ static int damon_reclaim_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 check expose a bug introduced earlier in this series?

If the user provides an invalid addr_unit (e.g., 3), min_region_sz will not be a power of 2. By removing the check here, this invalid parameter is passed to damon_commit_ctx().

Due to an issue in damon_commit_ctx() (from "mm/damon/core: safely validate src on damon_commit_ctx()"), the dry-run validates dst instead of src:

mm/damon/core.c:damon_commit_ctx() {
	...
	err = __damon_commit_ctx(test_ctx, dst);
	if (err)
		goto out;
	err = __damon_commit_ctx(dst, src);
	...
}

When __damon_commit_ctx() is called with the live dst and invalid src, it sets dst->maybe_corrupted before returning -EINVAL:

mm/damon/core.c:__damon_commit_ctx() {
	...
	dst->maybe_corrupted = true;
	if (!is_power_of_2(src->min_region_sz))
		return -EINVAL;
	...
}

Could this cause the background kdamond thread to see maybe_corrupted as true on its next iteration and permanently terminate, silently turning a simple configuration error into a denial of service for the subsystem?

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the is_power_of_2() validation exposes the system to undefined behavior via ALIGN macros and silently kills the kdamond thread (Denial of Service). --

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

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

DAMON_LRU_SORT validates the user input for min_region_sz. The same validation is done inside damon_start() and damon_commit_ctx(). Remove the unnecessary duplicate.

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 expose the rest of the DAMON subsystem to a non-power-of-two min_region_sz?

If a user writes a non-power-of-two module parameter (for example, addr_unit = 3), damon_lru_sort_apply_parameters() will calculate min_region_sz as 349525.

This invalid size is then passed to damon_set_region_system_rams_default(), and eventually to damon_set_regions(), where it is passed into the ALIGN() and ALIGN_DOWN() macros:

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));
    ...
}

These macros strictly require power-of-two alignment values to form bitwise masks.

Can this result in meaningless garbage boundaries?

Furthermore, the invalid size propagates to damon_commit_ctx(). Due to a bug in a prior patch in the series ('mm/damon/core: safely validate src on damon_commit_ctx()'), the safe validation dry-run incorrectly validates dst instead of src,

leading directly to a call to __damon_commit_ctx():

mm/damon/core.c:__damon_commit_ctx() {
    ...
    dst->maybe_corrupted = true;
    if (!is_power_of_2(src->min_region_sz))
        return -EINVAL;
    ...
}

This function immediately sets dst->maybe_corrupted = true, evaluates !is_power_of_2(src->min_region_sz), and returns -EINVAL early.

This leaves the running context's maybe_corrupted flag permanently set to true, which causes the kdamond background thread to abruptly terminate itself on its next iteration.

Could this lead to a denial of service by silently killing the kdamond thread?

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The documentation incorrectly claims damon_set_regions() validates region sizes, which led to the removal of input validation in callers, resulting in a kernel OOM panic when negative-sized regions are processed. --

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

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

This commit updates the documentation for the damon_region struct. It clarifies that callers can pass any values for region addresses because damon_set_regions() validates that regions have a positive size.

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

Quoted 10 lines

include/linux/damon.h +2 -1

@@ -51,7 +51,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]
Does this documentation accurately reflect the implementation?

Looking at damon_set_regions() in mm/damon/core.c, it lacks checks for start >= end before passing boundaries to damon_new_region():

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 a region is created with start >= end, would damon_sz_region() underflow to ULONG_MAX?

This enormous size could trigger an infinite memory allocation loop in damon_apply_min_nr_regions() which repeatedly calls damon_split_region_at():

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);
			}
    ...
}

Since a subsequent patch in the series (mm/damon/sysfs: remove region size validation) relies on this guarantee to remove the validation from sysfs, could a user writing a region with start >= end to the DAMON sysfs interface trigger an Out-of-Memory kernel panic?

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removal of bounds validation permits inverted regions (start > end), leading to a massive integer underflow and guaranteed OOM kernel panic. --

commit 03335779d391504c4265be49dedb155969cf1a7d
Author: SJ Park <[email protected]>

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

This commit removes the explicit start > end validation in damon_set_region_system_rams_default(), stating that damon_set_regions() already performs a complete check.

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

Quoted 11 lines

mm/damon/core.c +0 -3

index ace202f47dd7c..f491a76c98bfc 100644
@@ -3794,9 +3794,6 @@ int damon_set_region_system_rams_default(struct damon_target *t,
 {
 	struct damon_addr_range addr_range;
 
-	if (*start > *end)
-		return -EINVAL;
-

[Severity: High]
The commit message states that the complete check is done inside damon_set_regions(), but does damon_set_regions() actually validate that start is less than or equal to end?

Looking at damon_set_regions(), it appears to blindly create the region with underflowed sizes if start > end:

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 an inverted region (e.g., [4096, 0)) is passed in, could this lead to a massive integer underflow? The size of the region would underflow to roughly ULONG_MAX.

Later in damon_apply_min_nr_regions(), this underflow seems to translate into an effectively infinite 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);
        }
        ...
}

Could this loop exhaust system memory by attempting to allocate quadrillions of regions and cause an OOM kernel panic?

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the start > end validation allows users to configure monitoring regions with negative sizes, causing integer underflow in region size calculations. This leads to infinite memory allocation loops (OOM), potential list corruption, and CPU soft lockups during DAMON region splitting and scheme operations. --

commit 013d66b2108372ca2ada7fc6fcfd5e07f93fd78c
Author: SJ Park <[email protected]>

mm/damon/sysfs: remove region size validation

This commit removes the region size validation in DAMON sysfs that checks for negative size regions. The commit message states that the validation is incomplete and that damon_set_regions() already performs complete validation.

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

Quoted 10 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]
The commit message mentions that damon_set_regions() does the complete validation, but looking at damon_set_regions() in mm/damon/core.c, does it actually validate that start <= end?

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 a user configures a region where start > end, wouldn't this cause an integer underflow when calculating the region size later?

For example, could this underflow lead to an infinite memory allocation loop in 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);
	}

Could this loop evaluate the underflowed size and attempt to allocate new regions repeatedly until an OOM occurs?

Additionally, for newly split regions where start < end locally, could this spin over a massive address range without yielding and cause a CPU soft lockup in mm/damon/paddr.c:damon_pa_pageout():

	while (addr < damon_pa_phys_addr(r->ar.end, addr_unit)) {
		...
	}