Re: [PATCH] mm/damon/sysfs: preventing duplicated list_add_tail() at the damon_call()

[JaeJoon Jung <rgbi3307@gmail.com>]

On Thu, 25 Dec 2025 at 09:32, SeongJae Park <sj@kernel.org> wrote:
>
> Hello JaeJoon,
>
> On Wed, 24 Dec 2025 18:43:58 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:
>
> >  cd /sys/kernel/mm/damon/admin
> >  echo "off" > kdamonds/0/state
> >
> >  echo "commit" > kdamonds/0/state
> >  echo "commit" > kdamonds/0/state
> >
> > If you repeat "commit" twice with the kdamonds/0/state set to "off"
> > with the above command, list_add corruption error occurs as follows:
> >
> > 4-page vmalloc region starting at 0xffffffc600a38000 allocated at
> > kernel_clone+0x44/0x41e
> > ------------[ cut here ]------------
> > list_add corruption. prev->next should be next (ffffffd6c7c5a6a8),
> >          but was ffffffc600a3bcc8. (prev=ffffffc600a3bcc8).
> > WARNING: lib/list_debug.c:32 at __list_add_valid_or_report+
> > 0xd8/0xe2, CPU#0: bash/466
> > Modules linked in: dwmac_starfive stmmac_platform stmmac pcs_xpcs phylink
> > CPU: 0 UID: 0 PID: 466 Comm: bash Tainted: G W 6.19.0-rc2+ #1 PREEMPTLAZY
> > Tainted: [W]=WARN
> > Hardware name: StarFive VisionFive 2 v1.3B (DT)
> >  epc : __list_add_valid_or_report+0xd8/0xe2
> >  ra : __list_add_valid_or_report+0xd8/0xe2
> >  epc : ffffffff80540bce ra : ffffffff80540bce sp : ffffffc600a3bc00
> >  gp : ffffffff81caec40 tp : ffffffd6c036f080 t0 : 0000000000000000
> >  t1 : 0000000000006000 t2 : 0000000000000002 s0 : ffffffc600a3bc30
> >  s1 : ffffffc600a3bcc8 a0 : ffffffd6fbf49a40 a1 : ffffffd6c036f080
> >  a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000
> >  a5 : 0000000000000000 a6 : 0000000020000000 a7 : 0000000000000001
> >  s2 : ffffffd6c7c5a6a8 s3 : ffffffc600a3bcc8 s4 : ffffffc600a3bcc8
> >  s5 : ffffffd6c7c5a6b8 s6 : ffffffd6c7c5a6a8 s7 : 0000003ff3f32794
> >  s8 : 0000002ab38c9118 s9 : 0000000000000065 s10: 0000003f823a5cb8
> >  s11: 0000003f823264e8 t3 : 0000000000000001 t4 : 0000000000000000
> >  t5 : 00000000fa83b2da t6 : 000000000051df90
> > status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> > [<ffffffff80540bce>] __list_add_valid_or_report+0xd8/0xe2
> > [<ffffffff80255c86>] damon_call+0x52/0xe8
> > [<ffffffff8025c9a8>] damon_sysfs_damon_call+0x60/0x8a
> > [<ffffffff8025daf4>] state_store+0xfc/0x294
> > [<ffffffff80dbf1fa>] kobj_attr_store+0xe/0x1a
> > [<ffffffff802f070c>] sysfs_kf_write+0x42/0x56
> > [<ffffffff802eef4e>] kernfs_fop_write_iter+0xf4/0x178
> > [<ffffffff8026545c>] vfs_write+0x1b6/0x3b2
> > [<ffffffff80265782>] ksys_write+0x52/0xbc
> > [<ffffffff80265800>] __riscv_sys_write+0x14/0x1c
> > [<ffffffff80ddf124>] do_trap_ecall_u+0x19c/0x26e
> > [<ffffffff80deaa38>] handle_exception+0x150/0x15c
> > ---[ end trace 0000000000000000 ]---
> > -bash: echo: write error: Invalid argument
>
> Thank you for finding issue!
>
> Also appreciate for sharing your detailed reproducer.  Nevertheless, I think
> the reproducer can be more detailed.  E.g., you could explicitly explain the
> fact that the reproduction step should be executed only after starting DAMON
> with the kdamond, and the kernel should run with CONFIG_lIST_HARDENED to get
> the output from the kernel log.

Yes, as you said, I ran it under CONFIG_LIST_HARDENED=y condition.

>
> >
> > The cause of the above error is that list_add_tail() is executed
> > repeatedly while executing damon_call(ctx, control)
> > in damon_sysfs_damon_call().  The execution flow is summarized below:
> >
> > damon_sysfs_damon_call()
> >         --> damon_call(ctx, control)
> >                 list_add_tail(control, ctx->call_contols);
> >                         --> /* list_add corruption error */
> >                 if (!damon_is_running)
> >                         return -EINVAL;
> >
> > If you execute damon_call() when damon_sysfs_kdamond_running() is true,
> > you can prevent the error of duplicate execution of list_add_tail().
>
> The kdamond might be terminated between the damon_call() call and the
> damon_is_running() check inside the damon_call() execution.  In the case, the
> problem may still happen.
>
> The problem happens because damon_call() is not removing the damon_call_control
> object before returning the error, right?  What about removing the object
> before returning the error?

damon_call() is called after damon_start() --> kdamond_fn() is executed,
This is a problem because damon_call() also occurs when kdamond is "off"
only in damon/sysfs.  So, my first patch solved the problem, but the
following also worked. I tested it.

And it seems better to keep the existing method of releasing
damon_call_control.  Since the damon_call_control structure uses both
static and kmalloc(),  it's appropriate to release it in kdamond_fn() according
to the condition control->canceled && control->dealloc_on_cancel.

My previous suggestion regarding this:
https://lore.kernel.org/damon/20251206224724.13832-1-rgbi3307@gmail.com/

diff --git a/mm/damon/core.c b/mm/damon/core.c
index babad37719b6..2ead0bb3c462 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1462,6 +1462,9 @@ bool damon_is_running(struct damon_ctx *ctx)
  */
 int damon_call(struct damon_ctx *ctx, struct damon_call_control *control)
 {
+       if (!damon_is_running(ctx))
+               return -EINVAL;
+
        if (!control->repeat)
                init_completion(&control->completion);
        control->canceled = false;
@@ -1470,8 +1473,6 @@ int damon_call(struct damon_ctx *ctx, struct
damon_call_control *control)
        mutex_lock(&ctx->call_controls_lock);
        list_add_tail(&control->list, &ctx->call_controls);
        mutex_unlock(&ctx->call_controls_lock);
-       if (!damon_is_running(ctx))
-               return -EINVAL;
        if (control->repeat)
                return 0;
        wait_for_completion(&control->completion);

>
> >
> > Signed-off-by: JaeJoon Jung <rgbi3307@gmail.com>
>
> Could you please also add Fixes: and Cc: stable@ ?

I don't have much experience with this, so I'm sorry,
but could you please give me an example about this?

>
>
> Thanks,
> SJ
>
> [...]