Re: [RFC PATCH v2 2/2] mm/damon/lru_sort: error out for >10000 active_mem_bp

[sashiko-bot@kernel.org]

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

New issues:
- [High] The newly added validation check for `active_mem_bp` can be bypassed via a Time-Of-Check to Time-Of-Use (TOCTOU) race condition, resulting in division by zero.

Pre-existing issues:
- [High] The function `damos_get_in_active_mem_bp()` triggers a division by zero if the system tracks no active or inactive LRU pages.
- [High] Writing to the `commit_inputs` module parameter while DAMON is initialized but not running indefinitely suspends the writing process.
--

--- Patch [2]: [RFC PATCH v2 2/2] mm/damon/lru_sort: error out for >10000 active_mem_bp ---
commit e683bbcbac1ca0611d9909a3bb95074271d3aa04
Author: SJ Park <sj@kernel.org>

mm/damon/lru_sort: error out for >10000 active_mem_bp

damos_quota_score() can trigger division by zero if the target value is
zero.  DAMON_LRU_SORT lets users set the target value for the hot memory
scheme via active_mem_bp parameter.  It avoids setting it as the target
value if the parameter value is zero.  However, it also sets the cold
memory scheme with a target value that is calculated as '10000 -
active_mem_bp + 2'.  Hence, if a user sets active_mem_bp 10002, the cold
memory scheme's quota goal target value can be zero.  As a result,
division by zero can be triggered.  Fix by returning an error when the
user tries to start DAMON with >10000 active_mem_bp parameter value.

Link: https://lore.kernel.org/20260801213028.5127-1-sj@kernel.org

> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index 7e077084cb03a..3920d5042f109 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c
> @@ -233,6 +233,8 @@ static int damon_lru_sort_add_quota_goals(struct damos *hot_scheme,
>  
>  	if (!active_mem_bp)
>  		return 0;
> +	if (10000 < active_mem_bp)
> +		return -EINVAL;
>  	goal = damos_new_quota_goal(DAMOS_QUOTA_ACTIVE_MEM_BP, active_mem_bp);

[Severity: High]
Since active_mem_bp is a global module parameter, can its value change between
the bounds check and the call to damos_new_quota_goal()?

If a concurrent write updates active_mem_bp to 10002 immediately after the
check completes, the cold memory scheme's quota goal target value could still
end up being zero, resulting in the division by zero this patch aims to
prevent.

Would caching the value locally with READ_ONCE() avoid this race?

>  	if (!goal)
>  		return -ENOMEM;

While reviewing this code, I also noticed a couple of pre-existing issues.

This isn't a bug introduced by this patch, but in mm/damon/core.c:

mm/damon/core.c:damos_get_in_active_mem_bp() {
    ...
	active = global_node_page_state(NR_LRU_BASE + LRU_ACTIVE_ANON) +
		global_node_page_state(NR_LRU_BASE + LRU_ACTIVE_FILE);
	inactive = global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_ANON) +
		global_node_page_state(NR_LRU_BASE + LRU_INACTIVE_FILE);
	total = active + inactive;
	if (active_ratio)
		return mult_frac(active, 10000, total);
	return mult_frac(inactive, 10000, total);
}

[Severity: High]
Can mult_frac() trigger a division by zero if total is 0?

If the system tracks no active or inactive LRU pages, or if per-CPU counter
drifts clamp the global state to 0, total will evaluate to 0 and be used as
the denominator.


This is also a pre-existing issue, but in mm/damon/lru_sort.c:

mm/damon/lru_sort.c:damon_lru_sort_commit_inputs_store() {
    ...
	/*
	 * Skip damon_call() if ctx is not initialized to avoid
	 * NULL pointer dereference.
	 */
	if (!ctx)
		return -EINVAL;

	err = damon_call(ctx, &control);
    ...
}

[Severity: High]
Does damon_call() require the kdamond thread to be actively running?

If the module is initialized but DAMON has not been enabled, it appears
damon_call() will queue the request and wait indefinitely on a completion
that will never be signaled by the stopped thread, causing the writing
process to hang in an uninterruptible sleep state.

Could we check damon_is_running(ctx) before calling damon_call()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802162050.89477-1-sj@kernel.org?part=2