Re: [RFC PATCH v1.2 1/2] mm/damon/stat: add a parameter for reading kdamond pid

[SeongJae Park <sj@kernel.org>]

TL; DR: Sashiko found good point.  I will fix it in the next revision.

On Thu, 16 Apr 2026 00:59:35 +0000 sashiko-bot@kernel.org wrote:

> Sashiko AI review found 1 potential issue(s):
> - [Medium] Stale `kdamond_pid` exposed to userspace if `damon_call()` fails during startup.
> --
> 
> commit 46eb0a4278ccd2dec190db28b78b0766b8865901
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/stat: add a parameter for reading kdamond pid
> 
> This commit exposes the pid of the DAMON_STAT kdamond via a new read-only
> module parameter named kdamond_pid, standardizing usage with other DAMON
> modules.
> 
> > diff --git a/mm/damon/stat.c b/mm/damon/stat.c
> > index 99ba346f9e325..4901e9a7c3398 100644
> > --- a/mm/damon/stat.c
> > +++ b/mm/damon/stat.c
> 
> [ ... ]
> 
> > @@ -260,6 +269,14 @@ static int damon_stat_start(void)
> >  		damon_stat_context = NULL;
> >  		return err;
> >  	}
> > +	kdamond_pid = damon_kdamond_pid(damon_stat_context);
> > +	if (kdamond_pid < 0) {
> > +		err = kdamond_pid;
> > +		kdamond_pid = -1;
> > +		damon_destroy_ctx(damon_stat_context);
> > +		damon_stat_context = NULL;
> > +		return err;
> > +	}
> >  
> >  	damon_stat_last_refresh_jiffies = jiffies;
> >  	call_control.data = damon_stat_context;
> 
> If damon_call() fails right after this and returns an error,
> damon_stat_start() will pass that error back to its caller and the module
> will revert to a disabled state.
> 
> Does this leave a stale kdamond_pid exposed to userspace? The newly added
> documentation states: "If DAMON_STAT is enabled, this becomes the PID of
> the worker thread. Else, -1." Since the initialization fails, the module
> remains disabled, but kdamond_pid isn't reset to -1 on this error path.

Nice catch.  I will do the kdamond_pid setup after damon_call() success, like
below.  Also, I will remove the damon_stat_context deallocation and unset when
damon_kdamond_pid() failure, because it will be deallocated when
damon_stat_start() is called again.  The change will look like below:

--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -222,18 +222,19 @@ static int damon_stat_start(void)
                damon_stat_context = NULL;
                return err;
        }
+
+       damon_stat_last_refresh_jiffies = jiffies;
+       call_control.data = damon_stat_context;
+       err = damon_call(damon_stat_context, &call_control);
+       if (err)
+               return err;
+
        kdamond_pid = damon_kdamond_pid(damon_stat_context);
        if (kdamond_pid < 0) {
                err = kdamond_pid;
                kdamond_pid = -1;
-               damon_destroy_ctx(damon_stat_context);
-               damon_stat_context = NULL;
                return err;
        }
-
-       damon_stat_last_refresh_jiffies = jiffies;
-       call_control.data = damon_stat_context;
-       return damon_call(damon_stat_context, &call_control);
 }

 static void damon_stat_stop(void)

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


Thanks,
SJ