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, setting the active_mem_bp module parameter to 10002 results in a target_value of 0, triggering the same division by zero Oops in the core framework.
- [High] damos_new_quota_goal() allocates damos_quota_goal using kmalloc_obj(), leaving last_psi_total uninitialized. This causes wildcard auto-tuning behavior for PSI-based quota goals on their first cycle.
--

--- Patch [1]: [RFC PATCH] samples/damon/mtier: error out for zero quota goal 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] open here

samples/damon/mtier.c static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote) · line 161

Quoted 11 lines

samples/damon/mtier.c +3 -0

index ac9c24b92ead8..d1123ebbfab90 100644
@@ -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 user writes 10002 to the active_mem_bp parameter in the damon_lru_sort module, the cold scheme quota goal calculation will result in a zero target value:

mm/damon/lru_sort.c:damon_lru_sort_add_quota_goals() {
    ...
    goal = 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?

Quoted 5 lines

samples/damon/mtier.c +0 -0

 	quota_goal = 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_obj() without zeroing the memory:

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

This leaves union fields like last_psi_total uninitialized. When a PSI-based 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 = now_psi_total - goal->last_psi_total;
    ...
}

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

Quoted 3 lines

samples/damon/mtier.c +0 -0

 	if (!quota_goal)
 		goto free_out;

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1