mm/damon/core: validate ranges in damon_set_regions()

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

Pre-existing issues:
- [High] The validation check to prevent empty regions is bypassed due to integer overflow in ALIGN(), leaving the divide-by-zero vulnerability unpatched.
- [High] Memory allocation failure during region updates leaves the running DAMON context with a corrupted, partially updated region list. --

commit e67b2388e5d46f77944a06f1bd1d3d550638fade
Author: SeongJae Park <[email protected]>

mm/damon/core: validate ranges in damon_set_regions()

This patch updates damon_set_regions() to validate ranges and reject inputs where the start address is greater than or equal to the end address. This aims to prevent zero-length regions which can cause divide-by-zero errors in damon_merge_two_regions().

mm/damon/core.c int damon_set_regions(struct damon_target *t, struct damon_addr_range *ranges, · line 365

Quoted 13 lines

mm/damon/core.c +5 -0

index 7e4b9affc5b06..b3100d7fa5596 100644
@@ -358,6 +358,11 @@ int damon_set_regions(struct damon_target *t, struct damon_addr_range *ranges,
 	unsigned int i;
 	int err;
 
+	for (i = 0; i < nr_ranges; i++) {
+		if (ranges[i].start >= ranges[i].end)
+			return -EINVAL;
+	}
+

[Severity: High]
This isn't a bug introduced by this patch, but does this validation check sufficiently protect against overflow when the regions are aligned later?

If a user supplies a region where the end address is ULONG_MAX, ALIGN(ULONG_MAX, min_region_sz) will wrap around to 0.

Since this validation check uses unaligned inputs, ULONG_MAX passes the check here. Later, during allocation:

damon_set_regions() {
    ...
    r = damon_new_region(
            ALIGN_DOWN(ranges[i].start, min_region_sz),
            ALIGN(ranges[i].end, min_region_sz));
    ...
}

This creates a region with a wrapped and inverted size. If the adjacent regions sum to a size of 0, for example 4096 + (ULONG_MAX - 4095), can this lead to a divide-by-zero crash in damon_merge_two_regions() when the sizes are added together?

damon_merge_two_regions() {
    ...
    l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
            (sz_l + sz_r);
    ...
}

[Severity: High]
This is also a pre-existing issue, but what happens to the region list if memory allocation fails after existing regions are destroyed?

In damon_set_regions(), existing regions not in the new ranges are destroyed. If a subsequent damon_new_region() fails and returns -ENOMEM:

damon_set_regions() {
    ...
    if (!r)
        return -ENOMEM;
    damon_add_region(r, t);
    ...
}

The error rollback path in damon_revert_target_commits() does not restore the regions_list state:

damon_revert_target_commits() {
    ...
    damon_for_each_target(target, dst) {
        if (target == failed)
            return;
        put_pid(target->pid);
    }
}

Will this transient allocation failure permanently leave the running damon context with a corrupted or partially updated region list, silencing reclamation daemons?

DAMON core logic assumes zero length regions don't exist. However, a few DAMON API callers including DAMON_SYSFS, DAMON_RECLAIM and DAMON_LRU_SORT allow users to set empty monitoring target regions. This could result in WARN_ONCE() on CONFIG_DAMON_DEBUG_SANITY enabled kernel, and divide-by-zero from damon_merge_two_regions().

For example, the WANR_ONCE() can be triggered like below.

    # grep DAMON_DEBUG_SANITY /boot/config-$(uname -r)
    # CONFIG_DAMON_DEBUG_SANITY=y
    # damo start
    # cd /sys/kernel/mm/damon/admin/kdamonds/0
    # echo 0 > contexts/0/targets/0/regions/0/start
    # echo 0 > contexts/0/targets/0/regions/0/end
    # echo commit > state
    # dmesg
    [....]
    [   73.705780] ------------[ cut here ]------------
    [   73.707552] start 0 >= end 0
    [   73.708452] WARNING: mm/damon/core.c:359 at damon_new_region+0x6e/0x80, CPU#1: kdamond.0/758
    [...]

Disallow empty region user inputs by updating the validation logic.

Fixes 43b0536cb471 ("mm/damon: introduce DAMON-based Reclamation (DAMON_RECLAIM)")

Cc <[email protected]> # 5.16.x

Signed-off-by SeongJae Park <[email protected]>

---
 mm/damon/core.c | 5 +++++
 1 file changed, 5 insertions(+)

mm/damon/core.c +0 -0

index 7e4b9affc5b06..b3100d7fa5596 100644