Re: [RFC PATCH v2 1/2] samples/damon/mtier: handle damon_start() failure

[SeongJae Park <sj@kernel.org>]

On Tue, 09 Jun 2026 07:58:22 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Tue, 09 Jun 2026 14:35:38 +0000 sashiko-bot@kernel.org wrote:
> 
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > 
> > Pre-existing issues:
> > - [High] Resource leak (struct damon_ctx, struct damos, struct damon_target, and struct pid) on damon_start() failure.
> > --
> > 
> > commit e95eb25cfbc21e60927df5e7e4eadcf5468c1c41
> > Author: SeongJae Park <sj@kernel.org>
> > samples/damon/mtier: handle damon_start() failure
> > 
> > This commit fixes a resource leak in the memory tiering DAMON sample module
> > where failing to start the DAMON contexts leads to memory leaks and
> > indefinitely running contexts.
> > 
> > Link: https://lore.kernel.org/20260608112455.274231F00893@smtp.kernel.org
> > 
> > > diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
> > > --- a/samples/damon/mtier.c
> > > +++ b/samples/damon/mtier.c
> > 
> > [ ... ]
> > 
> > > @@ -185,7 +186,15 @@ static int damon_sample_mtier_start(void)
> > >  		return -ENOMEM;
> > >  	}
> > >  	ctxs[1] = ctx;
> > > -	return damon_start(ctxs, 2, true);
> > > +	err = damon_start(ctxs, 2, true);
> > > +	if (!err)
> > > +		return 0;
> > > +
> > > +	if (damon_is_running(ctxs[0]))
> > > +		damon_stop(ctxs, 1);
> > > +	damon_destroy_ctx(ctxs[0]);
> > > +	damon_destroy_ctx(ctxs[1]);
> > > +	return err;
> > >  }
> > 
> > [Severity: High]
> > This is a pre-existing issue, but it looks like the exact same resource leak
> > pattern exists in other DAMON sample modules when damon_start() fails.
> > 
> > Looking at damon_sample_prcl_start() in samples/damon/prcl.c:
> > 
> > 	err = damon_start(&ctx, 1, true);
> > 	if (err)
> > 		return err;
> > 
> > And similarly in damon_sample_wsse_start() in samples/damon/wsse.c:
> > 
> > 	err = damon_start(&ctx, 1, true);
> > 	if (err)
> > 		return err;
> > 
> > Since they return directly without calling damon_destroy_ctx() or putting
> > the target_pidp reference, do these modules also leak their newly allocated
> > contexts and PID references when damon_start() fails?
> 
> Nice finding.  Not a blocker of this patch, though.  I will separately work on
> this.

Hmm, but...  They do destroy the ctx in damon_sample_{wsse,prcl}_stop().  And
users cannot trigger damon_sample_{wsse,prcl}_start() without calling the stop
function first, due to the current 'enabled' value based skippinng in
damon_sample_wsse_enable_store().  So, this seems not a true issue.  Let me
know if I'm missing something.


Thanks,
SJ

[...]