Re: [PATCH 6.1.y] mm/damon/core: validate ranges in damon_set_regions()

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

New issues:
- [High] The validation check fails to consistently reject unaligned zero-l=
ength regions, allowing them to bypass validation and silently expand.
- [High] The ALIGN() macro overflows to 0 when evaluating regions near the =
end of the 32-bit address space, causing valid regions to be erroneously re=
jected.

Pre-existing issues:
- [High] Pre-existing leak of `struct pid` references if target addition or=
 context setup fails during sysfs input application.
--

--- Patch [1]: [PATCH 6.1.y] mm/damon/core: validate ranges in damon_set_re=
gions() ---
commit a1c890d67da57c98800fdd95b91daeac6ab475ab
Author: SJ Park <[email protected]>

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

This commit adds a validation check in damon_set_regions() to ensure that t=
he
provided region ranges are valid. It aims to prevent empty or negative leng=
th
regions from causing warnings and divide-by-zero errors in the core logic.

Link: https://lore.kernel.org/[email protected]

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 4b434ebd37c53..0a0bb033f28a4 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -212,6 +212,12 @@ int damon_set_regions(struct damon_target *t, struct=
 damon_addr_range *ranges,
>  	unsigned int i;
>  	int err;
> =20
> +	for (i =3D 0; i < nr_ranges; i++) {
> +		if (ALIGN_DOWN(ranges[i].start, DAMON_MIN_REGION) >=3D
> +				ALIGN(ranges[i].end, DAMON_MIN_REGION))

[Severity: High]
Does this logic successfully catch unaligned zero-length regions?

If a user configures a region with start and end both at an unaligned addre=
ss
like 4097, ALIGN_DOWN(4097) evaluates to 4096, and ALIGN(4097) evaluates to
8192. The check 4096 >=3D 8192 is false, which allows the invalid empty reg=
ion
to bypass validation and silently expand into a full page.

[Severity: High]
Will this ALIGN() check overflow on 32-bit systems when evaluating regions
near the end of the address space?

If the end address of the physical memory resource is ULONG_MAX (0xFFFFFFFF=
),
ALIGN(0xFFFFFFFF, 4096) overflows the 32-bit unsigned integer and evaluates
to 0. This makes the check ALIGN_DOWN(start) >=3D 0 unconditionally true, w=
hich
erroneously rejects valid memory regions and prevents DAMON initialization.

> +			return -EINVAL;
> +	}
> +
>  	/* Remove regions which are not in the new ranges */

[Severity: High]
This is a pre-existing issue, but does damon_destroy_ctx() leak struct pid
references if context setup fails?

Looking at damon_sysfs_build_ctx() in mm/damon/sysfs.c:

	err =3D damon_sysfs_apply_inputs(ctx, sys_ctx);
	if (err) {
		damon_destroy_ctx(ctx);
		return ERR_PTR(err);
	}

If damon_sysfs_apply_inputs() successfully adds targets (incrementing struct
pid refcounts) but a subsequent step fails, damon_destroy_ctx() is called.

Since vaddr ops do not define a .cleanup callback and the before_terminate
callback is never executed, the damon_target structures are freed without
calling put_pid(), permanently leaking the PID references.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260730010151.1190=
[email protected]?part=3D1