mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: SJ Park <sj@kernel.org>
To: SJ Park <sj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	damon@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, stable@vger.kernel.org
Subject: Re: [RFC PATCH 7.2.y] mm/damon/core: error damos_commit_quota_goal() for zero target_value
Date: Sun, 25 Oct 2026 11:00:39 -0700	[thread overview]
Message-ID: <20261025180040.170037-1-sj@kernel.org> (raw)
In-Reply-To: <20261025174647.165255-1-sj@kernel.org>

On Sun, 25 Oct 2026 10:46:46 -0700 SJ Park <sj@kernel.org> wrote:

> On Sun, 25 Oct 2026 10:24:33 -0700 SJ Park <sj@kernel.org> wrote:
> 
> > Patch series "mm/damon: move zero damos quota target_value handling to the
> > core layer".
> > 
> > Having zero DAMOS quota target value can cause division by zero.  DAMON
> > API callers are checking the target value parameters to avoid that.  It is
> > easy to make mistakes in some of the multiple API callers.  Move that to
> > the core layer.
> > 
> > Patch 1 adds the corner case handling into the core layer DAMON parameters
> > validation logic.  Patches 2 and 3 remove no more needed DAMON API callers
> > side handling of the corner case in DAMON_LRU_SORT and DMON_SAMPLIE_MTIER,
> > respectively.
> > 
> > This patch (of 3):
> > 
> > If a DAMOS scheme has a damos_quota_goal of zero target_value,
> > damos_quota_goal() could trigger division-by-zero error.  Hence each DAMON
> > API callers should do the zero target_value validation.  It is easy to
> > make mistakes.  Actually such bugs in DAMON_LRU_SORT and
> > DAMON_SAMPLE_MTIER were found and fixed [1].
> > 
> > It is better to handle the corner case only once in the core layer,
> > instead of multiple places in all DAMON API callers.  One straightforward
> > option is using an alternative denominator for the corner case in the
> > damos_quota_goal().  However, the zero target_value is meaningless.  In
> > this case, the quota goal is always evaluated as achieved or
> > over-achieved.  The quota will only keep being reduced.
> > 
> > Simply avoid using zero target_value by adding a check in the core layer
> > DAMOS quota goal parameters validation/commit path,
> > damos_commit_quota_goal().  Update it to return an error in the case.
> > Also update its caller to propagate the error.
> > 
> > Notes for stable kernel porting.  The upstream commit doesn't have
> > Fixes: and therefore not ported to stable kernels.  It turned out it
> > fixes commit 8b549a4fd3c5 ("mm/damon/sysfs-schemes: commit damos quota
> > goals user input to DAMOS") which was merged into v6.8.  The user impact
> > is a race condition that can result in divide-by-zero.  Hence backport.
> > Reproducing the issue requires sysfs write permission, certain DAMON ABI
> > usages that are supposed to be uncommon, and a rare race timing, though.
> > The issue was discovered [2] by Sashiko as a part [3] of its bug report.
> > 
> > [2] https://sashiko.dev/#/bug/linux-7f814c9a-986f-4103-a55a-ef9537dcf62b
> > [3] https://lore.kernel.org/20261021005234.94107-1-sj@kernel.org
> 
> On v7.2, DAMON sysfs interface calls damon_commit_ctx() for validation purpose,
> for parameters commit but initial DAMON start.  Hence this patch fixes the
> issue in parmeters commit but initial DAMON start.  I will post a new version
> to cover the start case.

For minimum change, the next version will add one more validation in DAMON
start path, like below.

'''
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -2226,6 +2226,20 @@ static int damon_sysfs_repeat_call_fn(void *data)
        return 0;
 }

+static bool damon_sysfs_valid_start_ctx(struct damon_ctx *ctx)
+{
+       struct damos *s;
+       struct damos_quota_goal *g;
+
+       damon_for_each_scheme(s, ctx) {
+               damos_for_each_quota_goal(g, &s->quota) {
+                       if (!g->targe_value)
+                               return false;
+               }
+       }
+       return true;
+}
+
 static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *kdamond)
 {
        struct damon_ctx *ctx;
@@ -2251,6 +2265,8 @@ static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *kdamond)
                kfree(repeat_call_control);
                return PTR_ERR(ctx);
        }
+       if (!damon_sysfs_valid_start_ctx(ctx))
+               return -EINVAL;
        err = damon_start(&ctx, 1, false);
        if (err) {
                kfree(repeat_call_control);
'''


Thanks,
SJ

[...]

      reply	other threads:[~2026-10-25 18:00 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-25 17:24 SJ Park
2026-10-25 17:46 ` SJ Park
2026-10-25 18:00   ` SJ Park [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261025180040.170037-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®