Re: [PATCH v2 4/5] mm/damon: introduce DAMOS_SPLIT action

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] TOCTOU race in damon_sysfs_mk_scheme() allows bypassing bounds validation for DAMOS_SPLIT target_order.
- [Medium] Uninitialized target_order field in damon_sysfs_scheme_alloc() leaks kernel memory to userspace.
- [Low] Unconditional use of HPAGE_PMD_ORDER causes a build failure when huge pages are disabled.
--

commit 3d569aafbbd14d480f96311f5d7fa6a898ce6974
Author: Wang Lian <lianux.mm@gmail.com>

mm/damon: introduce DAMOS_SPLIT action

Add DAMOS_SPLIT to the damos_action enum for splitting large folios
into smaller mTHP-order folios.  Add a target_order field to struct
damos to specify the desired split order.

> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 35b90b47e35a0..766984a1e63c5 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
[ ... ]
> @@ -571,8 +573,11 @@ struct damos {
>  	struct damos_access_pattern pattern;
>  	enum damos_action action;
>  	/*
> -	 * @target_order: target order for mTHP actions (DAMOS_COLLAPSE).
> -	 * 0 means system default (PMD order).  Valid: 0, 2..HPAGE_PMD_ORDER.
> +	 * @target_order: target mTHP order for DAMOS_COLLAPSE and
> +	 * DAMOS_SPLIT.  For COLLAPSE, 0 means PMD order default,
> +	 * valid values: 0, 2..HPAGE_PMD_ORDER.  For SPLIT,
> +	 * valid values: 2..HPAGE_PMD_ORDER-1; 0 and HPAGE_PMD_ORDER
> +	 * are rejected at scheme creation time (defaulting to 2).
>  	 */
>  	unsigned int target_order;
>  	unsigned long apply_interval_us;
> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index 085b8042d1250..00d502119c3a5 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c
[ ... ]
> @@ -3053,6 +3057,14 @@ static struct damos *damon_sysfs_mk_scheme(
>  			HPAGE_PMD_ORDER, HPAGE_PMD_ORDER);
>  		sysfs_scheme->target_order = 0;
>  	}
> +	if (sysfs_scheme->action == DAMOS_SPLIT &&
> +	    (sysfs_scheme->target_order == 0 ||
> +	     sysfs_scheme->target_order >= HPAGE_PMD_ORDER)) {

[Severity: Low]
When huge pages are disabled, HPAGE_PMD_ORDER can evaluate to a BUILD_BUG()
expression on some architectures. Will using it unconditionally here cause a
build failure during compile time?

> +		pr_warn("DAMON split: target_order %u invalid, need 2..%u. Defaulting to 2.\n",
> +			sysfs_scheme->target_order,
> +			HPAGE_PMD_ORDER - 1);
> +		sysfs_scheme->target_order = 2;
> +	}
>  	scheme->target_order = sysfs_scheme->target_order;

[Severity: Medium]
Could a concurrent sysfs write change sysfs_scheme->target_order between
the validation above and this assignment?

If sysfs_scheme->target_order is modified locklessly, it might bypass
the boundary checks and assign an invalid value to scheme->target_order.

[Severity: Medium]
It appears sysfs_scheme->target_order might be uninitialized when a scheme
is first allocated. In damon_sysfs_scheme_alloc(), the scheme structure is
allocated without zeroing:

mm/damon/sysfs-schemes.c:damon_sysfs_scheme_alloc() {
	...
	struct damon_sysfs_scheme *scheme = kmalloc_obj(*scheme);
	...
}

If target_order_show() reads this field before it is explicitly set:

mm/damon/sysfs-schemes.c:target_order_show() {
	...
	return sysfs_emit(buf, "%u\n", scheme->target_order);
}

Could this leak uninitialized kernel memory to userspace?

>  
>  	err = damos_sysfs_add_quota_score(sysfs_quotas->goals, &scheme->quota);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701114716.56503-1-lianux.mm@gmail.com?part=4