All of lore.kernel.org
 help / color / mirror / Atom feed
From: SJ Park <sj@kernel.org>
To: Xiaolei Wang <xiaoleiwangxiaolei@gmail.com>
Cc: SJ Park <sj@kernel.org>,
	akpm@linux-foundation.org, david@kernel.org, ljs@kernel.org,
	error27@gmail.com, damon@lists.linux.dev, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	"Liam R. Howlett" <liam@infradead.org>,
	Michal Hocko <mhocko@suse.com>, Mike Rapoport <rppt@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Vlastimil Babka <vbabka@kernel.org>,
	stable@vger.kernel.org
Subject: Re: [PATCH v2] mm/damon/tests/sysfs-kunit: fix leaked pid reference in add_targets test
Date: Thu, 29 Oct 2026 07:10:40 -0700	[thread overview]
Message-ID: <20261029141040.148392-1-sj@kernel.org> (raw)
In-Reply-To: <20261029075609.35661-1-xiaoleiwangxiaolei@gmail.com>

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@.

Cc: <stable@vger.kernel.org> # 6.7.x

> Cc: Liam R. Howlett <liam@infradead.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Suren Baghdasaryan <surenb@google.com>
> Cc: Vlastimil Babka <vbabka@kernel.org>
> Signed-off-by: Wang Xiaolei <xiaoleiwangxiaolei@gmail.com>

Other than Cc: stable@,

Reviewed-by: SJ Park <sj@kernel.org>

Assuming you agree to Cc stable@, I applied this patch to damon/next [1] tree
after adding the Cc.  I will repost it next Monday, as an mm.git merge request.
So, no action from your side is needed for now.

[1] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees


Thanks,
SJ

[...]

  parent reply	other threads:[~2026-10-29 14:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29  7:56 [PATCH v2] mm/damon/tests/sysfs-kunit: fix leaked pid reference in add_targets test Xiaolei Wang
2026-10-29  8:47 ` sashiko-bot
2026-10-29 14:10 ` SJ Park [this message]
2026-10-29 15:18   ` Dan Carpenter

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=20261029141040.148392-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --cc=david@kernel.org \
    --cc=error27@gmail.com \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=rppt@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    --cc=xiaoleiwangxiaolei@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.