linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: SeongJae Park <sj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"# 6 . 14 . x" <stable@vger.kernel.org>,
	damon@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, JaeJoon Jung <rgbi3307@gmail.com>
Subject: Re: [PATCH] mm/damon/core: remove call_control in inactive contexts
Date: Mon, 29 Dec 2025 17:45:30 -0800	[thread overview]
Message-ID: <20251230014532.47563-1-sj@kernel.org> (raw)
In-Reply-To: <20251228183105.289441-1-sj@kernel.org>

On Sun, 28 Dec 2025 10:31:01 -0800 SeongJae Park <sj@kernel.org> wrote:

> If damon_call() is executed against a DAMON context that is not running,
> the function returns error while keeping the damon_call_control object
> linked to the context's call_controls list.  Let's suppose the object is
> deallocated after the damon_call(), and yet another damon_call() is
> executed against the same context.  The function tries to add the new
> damon_call_control object to the call_controls list, which still has the
> pointer to the previous damon_call_control object, which is deallocated.
> As a result, use-after-free happens.
> 
> This can actually be triggered using the DAMON sysfs interface.  It is
> not easily exploitable since it requires the sysfs write permission and
> making a definitely weird file writes, though.  Please refer to the
> report for more details about the issue reproduction steps.
> 
> Fix the issue by making damon_call() to cleanup the damon_call_control
> object before returning the error.
> 
> Reported-by: JaeJoon Jung <rgbi3307@gmail.com>
> Closes: https://lore.kernel.org/20251224094401.20384-1-rgbi3307@gmail.com
> Fixes: 42b7491af14c ("mm/damon/core: introduce damon_call()")
> Cc: <stable@vger.kernel.org> # 6.14.x
> Signed-off-by: SeongJae Park <sj@kernel.org>
> ---
>  mm/damon/core.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 2d3e8006db50..65482a0ce20b 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1442,6 +1442,35 @@ bool damon_is_running(struct damon_ctx *ctx)
>  	return running;
>  }
>  
> +/*
> + * damon_call_handle_inactive_ctx() - handle DAMON call request that added to
> + *				      an inactive context.
> + * @ctx:	The inactive DAMON context.
> + * @control:	Control variable of the call request.
> + *
> + * This function is called in a case that @control is added to @ctx but @ctx is
> + * not running (inactive).  See if @ctx handled @control or not, and cleanup
> + * @control if it was not handled.
> + *
> + * Returns 0 if @control was handled by @ctx, negative error code otherwise.
> + */
> +static int damon_call_handle_inactive_ctx(
> +		struct damon_ctx *ctx, struct damon_call_control *control)
> +{
> +	struct damon_call_control *c;
> +
> +	mutex_lock(&ctx->call_controls_lock);
> +	list_for_each_entry(c, &ctx->call_controls, list) {
> +		if (c == control) {
> +			list_del(&control->list);
> +			mutex_unlock(&ctx->call_controls_lock);
> +			return -EINVAL;
> +		}
> +	}
> +	mutex_unlock(&ctx->call_controls_lock);
> +	return 0;
> +}
> +
>  /**
>   * damon_call() - Invoke a given function on DAMON worker thread (kdamond).
>   * @ctx:	DAMON context to call the function for.
> @@ -1472,7 +1501,7 @@ int damon_call(struct damon_ctx *ctx, struct damon_call_control *control)
>  	list_add_tail(&control->list, &ctx->call_controls);
>  	mutex_unlock(&ctx->call_controls_lock);
>  	if (!damon_is_running(ctx))
> -		return -EINVAL;
> +		return damon_call_handle_inactive_ctx(ctx, control);
>  	if (control->repeat)
>  		return 0;
>  	wait_for_completion(&control->completion);

TL; DR: This patch introduces another UAF bug under a race condition.  I will
send a new version of the fix that solves the another issue.  Andrew, could you
please remove this from mm tree for now?

kdamond_fn() resets ->kdamond, which is read by damon_is_running(), and then
make the final kdamond_call() for cancelling any remaining damon_call()
requests.  Hence, if the above damon_is_running() was invoked between the
->kdamond reset and the final kdamond_call() invocation,
damon_call_handle_inactive_ctx() and the final kdamond_call() could
concurrently run.

kdamond_call() safely get a pointer to a damon_call_control object in
ctx->call_controls, and then access it without a lock.  Only after that, it
removes the object from the list while holding the lock.  The intermediate
lock-less access is safe because kdamond_call() is the only code that removes
items from ctx->call_controls.  But this patch makes it no more safe, because
this patch is introducing another ctx->call_controls item removing code, namely
damon_call_handle_inactive_ctx().

To see this in details, let's suppose kdamond_call() got the pointer, and
released the call_controls_lock.  After that, damon_call_handle_inactive_ctx()
shows the object is still in the ctx->call_controls, and removes it from the
list.  The damon_call() caller further deallocates the object.  Then, continued
execution of kdamond_call() accesses the already deallocated object.

I will send a new version of this fix soon.


Thanks,
SJ

[...]


  reply	other threads:[~2025-12-30  1:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-28 18:31 SeongJae Park
2025-12-30  1:45 ` SeongJae Park [this message]
2025-12-30  2:41   ` SeongJae Park
2025-12-30  3:45     ` SeongJae Park
2025-12-31  1:25       ` SeongJae Park
2025-12-31  5:27         ` JaeJoon Jung
2025-12-31 15:26           ` SeongJae Park
2026-01-01  0:55             ` JaeJoon Jung
2026-01-01  1:41               ` SeongJae Park
2026-01-01  2:58                 ` JaeJoon Jung

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=20251230014532.47563-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rgbi3307@gmail.com \
    --cc=stable@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox