Re: [RFC PATCH] samples/damon/mtier: error out for zero quota goal target values
SJ Park <[email protected]> Sat, 1 Aug 2026 14:30:26 -0700
| Newsgroups | dev.linux.lists.damon |
|---|---|
| Message-ID | <[email protected]> |
On Sat, 01 Aug 2026 21:25:50 +0000 [email protected] 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 <[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 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/[email protected] > > > if (!quota_goal) > > goto free_out; > > -- > Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1 Thanks, SJ