[PATCH] mm/damon/core: avoid use of half-online-committed context

[PATCH] mm/damon/core: avoid use of half-online-committed context

[SeongJae Park]

One major usage of damon_call() is online DAMON parameters update.  It
is done by calling damon_commit_ctx() inside the damon_call() callback
function.  damon_commit_ctx() can fail for two reasons: 1) invalid
parameters and 2) internal memory allocation failures.  In case of
failures, the damon_ctx that attempted to be updated (commit
destination) can be partially updated (or, corrupted from a
perspective), and therefore shouldn't be used anymore.  The function
only ensures the damon_ctx object can safely deallocated using
damon_destroy_ctx().

The API callers are, however, calling damon_commit_ctx() only after
asserting the parameters are valid, to avoid damon_commit_ctx() fails
due to invalid input parameters.  But it can still theoretically fail if
the internal memory allocation fails.  In the case, DAMON may run with
the partially updated damon_ctx.  This can result in unexpected
behaviors including even NULL pointer dereference in case of
damos_commit_dests() failure [1].  Such allocation failure is arguably
too small to fail, so the real world impact would be rare.  But, given
the bad consequence, this needs to be fixed.

Avoid such partially-committed (maybe-corrupted) damon_ctx use by saving
the damon_commit_ctx() failure on the damon_ctx object.  For this,
introduce damon_ctx->maybe_corrupted field.  damon_commit_ctx() sets it
when it is failed.  kdamond_call() checks if the field is set after each
damon_call_control->fn() is executed.  If it is set, ignore remaining
callback requests and return.  All kdamond_call() callers including
kdamond_fn() also check the maybe_corrupted field right after
kdamond_call() invocations.  If the field is set, break the
kdamond_fn() main loop so that DAMON sill doesn't use the context that
might be corrupted.

[1] https://lore.kernel.org/20260319043309.97966-1-sj@kernel.org

Fixes: 3301f1861d34 ("mm/damon/sysfs: handle commit command using damon_call()")
Cc: <stable@vger.kernel.org> # 6.15.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/linux/damon.h | 6 ++++++
 mm/damon/core.c       | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index a4fea23da8576..be3d198043ff9 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -810,6 +810,12 @@ struct damon_ctx {
 	struct damos_walk_control *walk_control;
 	struct mutex walk_control_lock;
 
+	/*
+	 * indicate if this may be corrupted.  Currentonly this is set only for
+	 * damon_commit_ctx() failure.
+	 */
+	bool maybe_corrupted;
+
 	/* Working thread of the given DAMON context */
 	struct task_struct *kdamond;
 	/* Protects @kdamond field access */
diff --git a/mm/damon/core.c b/mm/damon/core.c
index c1d1091d307e4..37454e8c9c510 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1252,6 +1252,7 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
 {
 	int err;
 
+	dst->maybe_corrupted = true;
 	if (!is_power_of_2(src->min_region_sz))
 		return -EINVAL;
 
@@ -1277,6 +1278,7 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
 	dst->addr_unit = src->addr_unit;
 	dst->min_region_sz = src->min_region_sz;
 
+	dst->maybe_corrupted = false;
 	return 0;
 }
 
@@ -2678,6 +2680,8 @@ static void kdamond_call(struct damon_ctx *ctx, bool cancel)
 			complete(&control->completion);
 		else if (control->canceled && control->dealloc_on_cancel)
 			kfree(control);
+		if (ctx->maybe_corrupted)
+			break;
 	}
 
 	mutex_lock(&ctx->call_controls_lock);
@@ -2707,6 +2711,8 @@ static int kdamond_wait_activation(struct damon_ctx *ctx)
 		kdamond_usleep(min_wait_time);
 
 		kdamond_call(ctx, false);
+		if (ctx->maybe_corrupted)
+			return -EINVAL;
 		damos_walk_cancel(ctx);
 	}
 	return -EBUSY;
@@ -2790,6 +2796,8 @@ static int kdamond_fn(void *data)
 		 * kdamond_merge_regions() if possible, to reduce overhead
 		 */
 		kdamond_call(ctx, false);
+		if (ctx->maybe_corrupted)
+			break;
 		if (!list_empty(&ctx->schemes))
 			kdamond_apply_schemes(ctx);
 		else

base-commit: 969615b6b5d178009a87abf4e4292f90c098978e
-- 
2.47.3

Re: [PATCH] mm/damon/core: avoid use of half-online-committed context

[Andrew Morton]

On Thu, 19 Mar 2026 07:52:17 -0700 SeongJae Park <sj@kernel.org> wrote:

> One major usage of damon_call() is online DAMON parameters update.  It
> is done by calling damon_commit_ctx() inside the damon_call() callback
> function.  damon_commit_ctx() can fail for two reasons: 1) invalid
> parameters and 2) internal memory allocation failures.  In case of
> failures, the damon_ctx that attempted to be updated (commit
> destination) can be partially updated (or, corrupted from a
> perspective), and therefore shouldn't be used anymore.  The function
> only ensures the damon_ctx object can safely deallocated using
> damon_destroy_ctx().
> 
> The API callers are, however, calling damon_commit_ctx() only after
> asserting the parameters are valid, to avoid damon_commit_ctx() fails
> due to invalid input parameters.  But it can still theoretically fail if
> the internal memory allocation fails.  In the case, DAMON may run with
> the partially updated damon_ctx.  This can result in unexpected
> behaviors including even NULL pointer dereference in case of
> damos_commit_dests() failure [1].  Such allocation failure is arguably
> too small to fail, so the real world impact would be rare.  But, given
> the bad consequence, this needs to be fixed.
> 
> Avoid such partially-committed (maybe-corrupted) damon_ctx use by saving
> the damon_commit_ctx() failure on the damon_ctx object.  For this,
> introduce damon_ctx->maybe_corrupted field.  damon_commit_ctx() sets it
> when it is failed.  kdamond_call() checks if the field is set after each
> damon_call_control->fn() is executed.  If it is set, ignore remaining
> callback requests and return.  All kdamond_call() callers including
> kdamond_fn() also check the maybe_corrupted field right after
> kdamond_call() invocations.  If the field is set, break the
> kdamond_fn() main loop so that DAMON sill doesn't use the context that
> might be corrupted.

I guess you saw the AI review?
	https://sashiko.dev/#/patchset/20260319145218.86197-1-sj%40kernel.org

Re: [PATCH] mm/damon/core: avoid use of half-online-committed context

[SeongJae Park]

On Thu, 19 Mar 2026 19:48:49 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 19 Mar 2026 07:52:17 -0700 SeongJae Park <sj@kernel.org> wrote:
> 
> > One major usage of damon_call() is online DAMON parameters update.  It
> > is done by calling damon_commit_ctx() inside the damon_call() callback
> > function.  damon_commit_ctx() can fail for two reasons: 1) invalid
> > parameters and 2) internal memory allocation failures.  In case of
> > failures, the damon_ctx that attempted to be updated (commit
> > destination) can be partially updated (or, corrupted from a
> > perspective), and therefore shouldn't be used anymore.  The function
> > only ensures the damon_ctx object can safely deallocated using
> > damon_destroy_ctx().
> > 
> > The API callers are, however, calling damon_commit_ctx() only after
> > asserting the parameters are valid, to avoid damon_commit_ctx() fails
> > due to invalid input parameters.  But it can still theoretically fail if
> > the internal memory allocation fails.  In the case, DAMON may run with
> > the partially updated damon_ctx.  This can result in unexpected
> > behaviors including even NULL pointer dereference in case of
> > damos_commit_dests() failure [1].  Such allocation failure is arguably
> > too small to fail, so the real world impact would be rare.  But, given
> > the bad consequence, this needs to be fixed.
> > 
> > Avoid such partially-committed (maybe-corrupted) damon_ctx use by saving
> > the damon_commit_ctx() failure on the damon_ctx object.  For this,
> > introduce damon_ctx->maybe_corrupted field.  damon_commit_ctx() sets it
> > when it is failed.  kdamond_call() checks if the field is set after each
> > damon_call_control->fn() is executed.  If it is set, ignore remaining
> > callback requests and return.  All kdamond_call() callers including
> > kdamond_fn() also check the maybe_corrupted field right after
> > kdamond_call() invocations.  If the field is set, break the
> > kdamond_fn() main loop so that DAMON sill doesn't use the context that
> > might be corrupted.
> 
> I guess you saw the AI review?
> 	https://sashiko.dev/#/patchset/20260319145218.86197-1-sj%40kernel.org

Yes :)  Pasting it and my reply in line below.  TL; DR: We need a fixup that
attached below.

: > --- a/mm/damon/core.c
: > +++ b/mm/damon/core.c
: > @@ -1252,6 +1252,7 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
: >  {
: >       int err;
: >
: > +     dst->maybe_corrupted = true;
: >       if (!is_power_of_2(src->min_region_sz))
: >               return -EINVAL;
: 
: Could this prematurely mark the context as corrupted?
: 
: If the power-of-2 validation fails, the function returns -EINVAL without
: modifying dst at all. However, because maybe_corrupted is already set to
: true, wouldn't the kdamond thread assume its state is corrupted and
: unconditionally shut down?
: 
: Should the flag be set immediately prior to the first mutating operation
: like damon_commit_schemes() instead?

Good suggestion.  But marking it as corrupted here doesn't make something
really bad.  Meanwhile, ksensKeep being aware of what function here is really
making change to the struct will make it complicated to maintain in long term.
I'd prefer keeping the code as is for the simplicity.

: 
: [ ... ]
: 
: > @@ -2678,6 +2680,8 @@ static void kdamond_call(struct damon_ctx *ctx, bool cancel)
: >                       complete(&control->completion);
: >               else if (control->canceled && control->dealloc_on_cancel)
: >                       kfree(control);
: > +             if (ctx->maybe_corrupted)
: > +                     break;
: >       }
: 
: Does this early break cause a deadlock during cancellation?
: 
: When damon_commit_ctx() fails, it sets ctx->maybe_corrupted = true.
: This corruption triggers an exit from the main loop in kdamond_fn() to the
: done: label, which invokes kdamond_call(ctx, true) to cancel and complete
: all remaining pending callbacks.
: 
: However, inside kdamond_call(), this loop now unconditionally checks
: if (ctx->maybe_corrupted) break; at the end of each iteration. Because
: the flag is true, won't the loop process exactly one callback and then
: break, leaving the rest of the callbacks in ctx->call_controls unprocessed?
: 
: If the worker thread then exits, wouldn't any callers waiting on
: wait_for_completion(&control->completion) for those remaining callbacks
: hang indefinitely?
: 
: Would it make sense to bypass this break condition when cancel is true?

Good catch!  Andrew, could you please add below attaching fixup?


Thanks,
SJ

[...]
=== >8 ===
From cafd5152c6d42e4153c54569b44b249e27939ad6 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj@kernel.org>
Date: Thu, 19 Mar 2026 20:09:27 -0700
Subject: [PATCH] mm/damon/core: let kdamond_call() with cancel regardless of
 maybe_corrupted

Otherwise, damon_call() callers could indefinitely wait [1].

[1] https://sashiko.dev/#/patchset/20260319145218.86197-1-sj%40kernel.org

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 37454e8c9c510..3e1890d64d067 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2680,7 +2680,7 @@ static void kdamond_call(struct damon_ctx *ctx, bool cancel)
 			complete(&control->completion);
 		else if (control->canceled && control->dealloc_on_cancel)
 			kfree(control);
-		if (ctx->maybe_corrupted)
+		if (!cancel && ctx->maybe_corrupted)
 			break;
 	}
 
-- 
2.47.3

Re: [PATCH] mm/damon/core: avoid use of half-online-committed context

[SeongJae Park]

On Thu, 19 Mar 2026 19:48:49 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 19 Mar 2026 07:52:17 -0700 SeongJae Park <sj@kernel.org> wrote:
> 
> > One major usage of damon_call() is online DAMON parameters update.  It
> > is done by calling damon_commit_ctx() inside the damon_call() callback
> > function.  damon_commit_ctx() can fail for two reasons: 1) invalid
> > parameters and 2) internal memory allocation failures.  In case of
> > failures, the damon_ctx that attempted to be updated (commit
> > destination) can be partially updated (or, corrupted from a
> > perspective), and therefore shouldn't be used anymore.  The function
> > only ensures the damon_ctx object can safely deallocated using
> > damon_destroy_ctx().
> > 
> > The API callers are, however, calling damon_commit_ctx() only after
> > asserting the parameters are valid, to avoid damon_commit_ctx() fails
> > due to invalid input parameters.  But it can still theoretically fail if
> > the internal memory allocation fails.  In the case, DAMON may run with
> > the partially updated damon_ctx.  This can result in unexpected
> > behaviors including even NULL pointer dereference in case of
> > damos_commit_dests() failure [1].  Such allocation failure is arguably
> > too small to fail, so the real world impact would be rare.  But, given
> > the bad consequence, this needs to be fixed.
> > 
> > Avoid such partially-committed (maybe-corrupted) damon_ctx use by saving
> > the damon_commit_ctx() failure on the damon_ctx object.  For this,
> > introduce damon_ctx->maybe_corrupted field.  damon_commit_ctx() sets it
> > when it is failed.  kdamond_call() checks if the field is set after each
> > damon_call_control->fn() is executed.  If it is set, ignore remaining
> > callback requests and return.  All kdamond_call() callers including
> > kdamond_fn() also check the maybe_corrupted field right after
> > kdamond_call() invocations.  If the field is set, break the
> > kdamond_fn() main loop so that DAMON sill doesn't use the context that
> > might be corrupted.
> 
> I guess you saw the AI review?
> 	https://sashiko.dev/#/patchset/20260319145218.86197-1-sj%40kernel.org

By the way, I am also doing monitoring of sashiko.dev for all DAMON patches.
It will be much easier once sashiko.dev's email feature is ready, since I
already onboarded DAMON for that.

Meanwhile, the monitoring using web browser is somewhat tedious for me, so I
just implemented an hkml feature, namely
'hkml patch sashiko_dev --thread_status'.  It receives a message id of a mail,
and prints the review status/result of all patches of the thread.

E.g.,

    $ hkml patch sashiko_dev --thread_status 20260319-memory-failure-mf-delayed-fix-rfc-v2-v2-0-92c596402a7a@google.com
    - [PATCH RFC v2 1/7] mm: memory_failure: Clarify the MF_DELAYED definition
      - Reviewed (Review completed successfully.)
    - [PATCH RFC v2 2/7] mm: memory_failure: Allow truncate_error_folio to return MF_DELAYED
      - Reviewed (Review completed successfully.)
    - [PATCH RFC v2 3/7] mm: shmem: Update shmem handler to the MF_DELAYED definition
      - Reviewed (Review completed successfully.)
    - [PATCH RFC v2 4/7] mm: memory_failure: Generalize extra_pins handling to all MF_DELAYED cases
      - Pending (None)
    - [PATCH RFC v2 4/7] mm: memory_failure: Generalize extra_pins handling to all MF_DELAYED cases
      - Reviewed (Review completed successfully.)
    - [PATCH RFC v2 5/7] mm: selftests: Add shmem memory failure test
      - Reviewed (Review completed successfully.)
    - [PATCH RFC v2 6/7] KVM: selftests: Add memory failure tests in guest_memfd_test
      - Reviewed (Review completed successfully.)
    - [PATCH RFC v2 7/7] KVM: selftests: Test guest_memfd behavior with respect to stage 2 page tables
      - Reviewed (Review completed successfully.)

I'm planning to implement another feature for formatting and sending the review
result and inline comments as emails, probably this weekend.


Thanks,
SJ

Re: [PATCH] mm/damon/core: avoid use of half-online-committed context

[SeongJae Park]

On Fri, 20 Mar 2026 19:16:27 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Thu, 19 Mar 2026 19:48:49 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Thu, 19 Mar 2026 07:52:17 -0700 SeongJae Park <sj@kernel.org> wrote:
> > 
> > > One major usage of damon_call() is online DAMON parameters update.  It
> > > is done by calling damon_commit_ctx() inside the damon_call() callback
> > > function.  damon_commit_ctx() can fail for two reasons: 1) invalid
> > > parameters and 2) internal memory allocation failures.  In case of
> > > failures, the damon_ctx that attempted to be updated (commit
> > > destination) can be partially updated (or, corrupted from a
> > > perspective), and therefore shouldn't be used anymore.  The function
> > > only ensures the damon_ctx object can safely deallocated using
> > > damon_destroy_ctx().
> > > 
> > > The API callers are, however, calling damon_commit_ctx() only after
> > > asserting the parameters are valid, to avoid damon_commit_ctx() fails
> > > due to invalid input parameters.  But it can still theoretically fail if
> > > the internal memory allocation fails.  In the case, DAMON may run with
> > > the partially updated damon_ctx.  This can result in unexpected
> > > behaviors including even NULL pointer dereference in case of
> > > damos_commit_dests() failure [1].  Such allocation failure is arguably
> > > too small to fail, so the real world impact would be rare.  But, given
> > > the bad consequence, this needs to be fixed.
> > > 
> > > Avoid such partially-committed (maybe-corrupted) damon_ctx use by saving
> > > the damon_commit_ctx() failure on the damon_ctx object.  For this,
> > > introduce damon_ctx->maybe_corrupted field.  damon_commit_ctx() sets it
> > > when it is failed.  kdamond_call() checks if the field is set after each
> > > damon_call_control->fn() is executed.  If it is set, ignore remaining
> > > callback requests and return.  All kdamond_call() callers including
> > > kdamond_fn() also check the maybe_corrupted field right after
> > > kdamond_call() invocations.  If the field is set, break the
> > > kdamond_fn() main loop so that DAMON sill doesn't use the context that
> > > might be corrupted.
> > 
> > I guess you saw the AI review?
> > 	https://sashiko.dev/#/patchset/20260319145218.86197-1-sj%40kernel.org
> 
> By the way, I am also doing monitoring of sashiko.dev for all DAMON patches.
> It will be much easier once sashiko.dev's email feature is ready, since I
> already onboarded DAMON for that.
> 
> Meanwhile, the monitoring using web browser is somewhat tedious for me, so I
> just implemented an hkml feature, namely
> 'hkml patch sashiko_dev --thread_status'.  It receives a message id of a mail,
> and prints the review status/result of all patches of the thread.
> 
> E.g.,
> 
>     $ hkml patch sashiko_dev --thread_status 20260319-memory-failure-mf-delayed-fix-rfc-v2-v2-0-92c596402a7a@google.com
>     - [PATCH RFC v2 1/7] mm: memory_failure: Clarify the MF_DELAYED definition
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 2/7] mm: memory_failure: Allow truncate_error_folio to return MF_DELAYED
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 3/7] mm: shmem: Update shmem handler to the MF_DELAYED definition
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 4/7] mm: memory_failure: Generalize extra_pins handling to all MF_DELAYED cases
>       - Pending (None)
>     - [PATCH RFC v2 4/7] mm: memory_failure: Generalize extra_pins handling to all MF_DELAYED cases
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 5/7] mm: selftests: Add shmem memory failure test
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 6/7] KVM: selftests: Add memory failure tests in guest_memfd_test
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 7/7] KVM: selftests: Test guest_memfd behavior with respect to stage 2 page tables
>       - Reviewed (Review completed successfully.)
> 
> I'm planning to implement another feature for formatting and sending the review
> result and inline comments as emails, probably this weekend.

I wanted to add the link to the commit that implementing the feature, but
forgot that, sorry.  Here it is:
https://github.com/sjp38/hackermail/commit/cf1b4e167067e5684823137c5296dfb268364175

FWIW, iiuc, 'b4 review' should also provide similar or better feature.


Thanks,
SJ

[...]

Re: [PATCH] mm/damon/core: avoid use of half-online-committed context

[SeongJae Park]

On Fri, 20 Mar 2026 19:16:35 SeongJae Park <sj@kernel.org> wrote:
[...]
> By the way, I am also doing monitoring of sashiko.dev for all DAMON patches.
> It will be much easier once sashiko.dev's email feature is ready, since I
> already onboarded DAMON for that.
> 
> Meanwhile, the monitoring using web browser is somewhat tedious for me, so I
> just implemented an hkml feature, namely
> 'hkml patch sashiko_dev --thread_status'.  It receives a message id of a mail,
> and prints the review status/result of all patches of the thread.
> 
> E.g.,
> 
>     $ hkml patch sashiko_dev --thread_status 20260319-memory-failure-mf-delayed-fix-rfc-v2-v2-0-92c596402a7a@google.com
>     - [PATCH RFC v2 1/7] mm: memory_failure: Clarify the MF_DELAYED definition
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 2/7] mm: memory_failure: Allow truncate_error_folio to return MF_DELAYED
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 3/7] mm: shmem: Update shmem handler to the MF_DELAYED definition
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 4/7] mm: memory_failure: Generalize extra_pins handling to all MF_DELAYED cases
>       - Pending (None)
>     - [PATCH RFC v2 4/7] mm: memory_failure: Generalize extra_pins handling to all MF_DELAYED cases
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 5/7] mm: selftests: Add shmem memory failure test
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 6/7] KVM: selftests: Add memory failure tests in guest_memfd_test
>       - Reviewed (Review completed successfully.)
>     - [PATCH RFC v2 7/7] KVM: selftests: Test guest_memfd behavior with respect to stage 2 page tables
>       - Reviewed (Review completed successfully.)
> 
> I'm planning to implement another feature for formatting and sending the review
> result and inline comments as emails, probably this weekend.

Now the feature is available on 'master' branch of hkml.  I started using it
since yesterday for DAMON patches, and it works for at least my workflow.  The
documentation is also updated [1].

[1] https://github.com/sjp38/hackermail/blob/test/USAGE.md#forwarding-sashikodev-statuscomments-to-mailing-list


Thanks,
SJ

[...]

Re: [PATCH] mm/damon/core: avoid use of half-online-committed context

[SeongJae Park]

On Sun, 22 Mar 2026 12:26:48 SeongJae Park <sj@kernel.org> wrote:
[...]
> Now the feature is available on 'master' branch of hkml.  I started using it
> since yesterday for DAMON patches, and it works for at least my workflow.  The
> documentation is also updated [1].
> 
> [1] https://github.com/sjp38/hackermail/blob/test/USAGE.md#forwarding-sashikodev-statuscomments-to-mailing-list

The above link is broken, sorry.  Please use below.

https://github.com/sjp38/hackermail/blob/master/USAGE.md#sashikodev


Thanks,
SJ

[...]