All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] samples/damon/mtier: error out for zero quota goal target values
@ 2026-08-01 21:13 SJ Park
  2026-08-01 21:25 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: SJ Park @ 2026-08-01 21:13 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, Yunjeong Mun, damon, linux-kernel,
	linux-mm

damos_quota_score() can trigger division by zero if the target_value is
zero.  DAMON_SAMPLE_MTIER lets users set the target_value via
node0_mem_{used,free}_bp parameters.  It doesn't guard zero value case,
though.  As a result, users can trigger division by zero.  Fix the issue
by returning an error when the user tries to start DAMON with zero
node0_mem_{used,free}_bp parameter values.

DAMON_SAMPLE_MTIER is just a sample module, but the consequence is quite
bad.  Also the zero node0_mem_free_bp parameter might look like a
reasonable setup to some users.  Hence, the issue might really happen in
the real world.

One reliable way to reproduce the issue is like below:

    # cd /sys/module/damon_sample_mtier/parameters
    # echo 4096 > node0_start_addr
    # echo 8192 > node0_end_addr
    # echo 8192 > node1_start_addr
    # echo 81920 > node1_end_addr
    # echo 0 > node0_mem_free_bp
    # echo Y > enabled
    # dmesg -w
    [...]
    [18792.235916] Oops: divide error: 0000 [#1] SMP NOPTI
    [...]
    [18792.242787] RIP: 0010:damos_quota_score+0x6f/0x480
    [...]

This issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260801202657.117135-1-sj@kernel.org

Fixes: c5e67d40a102 ("samples/damon/mtier: add parameters for node0 memory usage")
Cc: <stable@vger.kernel.org> # 6.17.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 samples/damon/mtier.c | 3 +++
 1 file changed, 3 insertions(+)

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;
 	quota_goal = damos_new_quota_goal(
 			promote ? DAMOS_QUOTA_NODE_MEM_USED_BP :
 			DAMOS_QUOTA_NODE_MEM_FREE_BP,

base-commit: 51b0df5cb9a4175af0e3c9e471cd70e91ff8eb4b
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] samples/damon/mtier: error out for zero quota goal target values
  2026-08-01 21:13 [RFC PATCH] samples/damon/mtier: error out for zero quota goal target values SJ Park
@ 2026-08-01 21:25 ` sashiko-bot
  2026-08-01 21:30   ` SJ Park
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-08-01 21:25 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

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 <sj@kernel.org>

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/20260801202657.117135-1-sj@kernel.org

> 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 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?

>  	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?

>  	if (!quota_goal)
>  		goto free_out;

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] samples/damon/mtier: error out for zero quota goal target values
  2026-08-01 21:25 ` sashiko-bot
@ 2026-08-01 21:30   ` SJ Park
  0 siblings, 0 replies; 3+ messages in thread
From: SJ Park @ 2026-08-01 21:30 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 01 Aug 2026 21:25:50 +0000 sashiko-bot@kernel.org wrote:

> 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 <sj@kernel.org>
> 
> 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/20260801202657.117135-1-sj@kernel.org
> 
> > 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 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?

Good catch.  Eventually I will fix this in core.  But let's make
easy-to-backport hotfixes for stable kernels at the moment.  I will add a fix
for lru_sort.

> 
> >  	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?

A fix [1] of the bug is under review.

[1] https://lore.kernel.org/20260801173554.94710-3-sj@kernel.org

> 
> >  	if (!quota_goal)
> >  		goto free_out;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801211315.2456-1-sj@kernel.org?part=1


Thanks,
SJ

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-01 21:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-01 21:13 [RFC PATCH] samples/damon/mtier: error out for zero quota goal target values SJ Park
2026-08-01 21:25 ` sashiko-bot
2026-08-01 21:30   ` SJ Park

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.