Re: [PATCH v2] mm/damon/tests/sysfs-kunit: fix leaked pid reference in add_targets test

[SJ Park <sj@kernel.org>]

On Thu, 29 Oct 2026 18:18:05 +0300 Dan Carpenter <error27@gmail.com> wrote:

> On Thu, Oct 29, 2026 at 07:10:40AM -0700, SJ Park wrote:
> > Hello Xiaolei,
> > 
> > On Thu, 29 Oct 2026 15:56:09 +0800 Xiaolei Wang <xiaoleiwangxiaolei@gmail.com> wrote:
> > 
> > > From: Wang Xiaolei <xiaoleiwangxiaolei@gmail.com>
> > > 
> > > damon_sysfs_test_add_targets() creates its monitoring context with
> > > damon_new_ctx(), which leaves ctx->ops zero-initialized.  Since
> > > DAMON_OPS_VADDR is zero, damon_target_has_pid() returns true for the
> > > context, and damon_sysfs_add_targets() therefore calls
> > > damon_set_target_pid() for each target it adds.  That function grabs a
> > > reference to the target's struct pid via find_get_pid().
> > > 
> > > The test then tears the context down with damon_destroy_ctx(), which
> > > releases such a reference from ctx->ops.cleanup_target().  For the vaddr
> > > operations set that callback is damon_va_cleanup_target(), and it is the
> > > only place where the reference is put.  The test never selects an
> > > operations set, though, so cleanup_target is NULL and both references --
> > > one for each of the two damon_sysfs_add_targets() calls -- are leaked on
> > > every run of the test.  Running the test with CONFIG_DEBUG_KMEMLEAK
> > > enabled reports the leaked struct pid objects as unreferenced.
> > > 
> > > Fix this by selecting the vaddr operations set for the context before
> > > adding the targets, mirroring what the sysfs interface does.
> > > damon_select_ops() fails if the vaddr operations set is not registered,
> > > in which case the test cannot exercise the pid handling and skips
> > > itself.
> > 
> > Makes sense.  Thank you for finding and fixing this.
> > 
> > > 
> > > Fixes: b8ee5575f763 ("mm/damon/sysfs-test: add a unit test for damon_sysfs_set_targets()")
> > 
> > The impact is leak.  I think we should add Cc: stable@.
> 
> I wouldn't bother Cc stable for unit tests.  It's not something
> which affects production or real life at all.

Thank you for your input, Dan!

I have no strong opinion.  But we got CVEs [1] for this class of unit test code
in the past.  I'd like to keep it consistent to the past for now.

[1] https://lore.kernel.org/2024102930-CVE-2024-50068-2e3c@gregkh/


Thanks,
SJ

[...]