Re: [RFC PATCH] samples/damon/mtier: error out for zero quota goal target values

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

Pre-existing issues:
- [High] The patch addresses a division by zero in the mtier sample module =
but leaves the core vulnerability unpatched. In `mm/damon/lru_sort.c`, sett=
ing the `active_mem_bp` module parameter to 10002 results in a `target_valu=
e` of 0, triggering the same division by zero Oops in the core framework.
- [High] `damos_new_quota_goal()` allocates `damos_quota_goal` using `kmall=
oc_obj()`, leaving `last_psi_total` uninitialized. This causes wildcard aut=
o-tuning behavior for PSI-based quota goals on their first cycle.
--

--- Patch [1]: [RFC PATCH] samples/damon/mtier: error out for zero quota go=
al target values ---
commit 1b5b3916c23bf9b477a9c0308b9b7d82d8503f36
Author: SJ Park <[email protected]>

samples/damon/mtier: error out for zero quota goal target values

This commit modifies the DAMON mtier sample to return an error when started
with a zero value for node0_mem_used_bp or node0_mem_free_bp. This prevents=
 a
division by zero Oops in damos_quota_score() when target_value is zero.

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

> diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
> index ac9c24b92ead8..d1123ebbfab90 100644
> --- a/samples/damon/mtier.c
> +++ b/samples/damon/mtier.c
> @@ -156,6 +156,9 @@ static struct damon_ctx *damon_sample_mtier_build_ctx=
(bool promote)
>  	if (!scheme)
>  		goto free_out;
>  	damon_set_schemes(ctx, &scheme, 1);
> +	/* zero target value causes division by zero in damos_quota_store() */
> +	if (!node0_mem_used_bp || !node0_mem_free_bp)
> +		goto free_out;

[Severity: High]
This isn't a bug introduced by this patch, but does this fix leave the core
vulnerability unpatched?

Since the underlying issue is in damos_quota_score() when target_value is 0:

mm/damon/core.c:damos_quota_score() {
    ...
    mult_frac(goal->current_value, 10000, goal->target_value)
    ...
}

It appears other modules can still trigger this Oops.  For example, if a us=
er
writes 10002 to the active_mem_bp parameter in the damon_lru_sort module, t=
he
cold scheme quota goal calculation will result in a zero target value:

mm/damon/lru_sort.c:damon_lru_sort_add_quota_goals() {
    ...
    goal =3D damos_new_quota_goal(DAMOS_QUOTA_INACTIVE_MEM_BP,
                                10000 - active_mem_bp + 2);
    ...
}

Should the zero check be handled centrally in damos_new_quota_goal() or
damos_quota_score() to prevent similar crashes from other modules?

>  	quota_goal =3D damos_new_quota_goal(
>  			promote ? DAMOS_QUOTA_NODE_MEM_USED_BP :
>  			DAMOS_QUOTA_NODE_MEM_FREE_BP,
>  			promote ? node0_mem_used_bp : node0_mem_free_bp);

[Severity: High]
This is a pre-existing issue, but does damos_new_quota_goal() leave fields
uninitialized?

Looking at damos_new_quota_goal(), it allocates the struct using kmalloc_ob=
j()
without zeroing the memory:

mm/damon/core.c:damos_new_quota_goal() {
    ...
    goal =3D kmalloc_obj(*goal);
    if (!goal)
        return NULL;
    goal->metric =3D metric;
    goal->target_value =3D target_value;
    INIT_LIST_HEAD(&goal->list);
    return goal;
}

This leaves union fields like last_psi_total uninitialized.  When a PSI-bas=
ed
quota goal is evaluated for the first time, this uninitialized value is used
in the feedback loop calculation:

mm/damon/core.c:damos_set_quota_goal_current_value() {
    ...
    goal->current_value =3D now_psi_total - goal->last_psi_total;
    ...
}

Could this uninitialized memory usage corrupt the auto-tuning feedback loop?

>  	if (!quota_goal)
>  		goto free_out;

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