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

PATCH In review SeongJae Park 2 participants 7 messages Mar 19 – Mar 22

Subsystems (3)

Stable branch Memory management DAMON

lore

Share & tools

lore

SeongJae Park

author maintainer

2026-03-19 14:52 UTC #0 lore raw

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

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/[email protected]

Fixes 3301f1861d34 ("mm/damon/sysfs: handle commit command using damon_call()")

Cc <[email protected]> # 6.15.x

Signed-off-by SeongJae Park <[email protected]>

---
 include/linux/damon.h | 6 ++++++
 mm/damon/core.c       | 8 ++++++++
 2 files changed, 14 insertions(+)

include/linux/damon.h +6 -0

index a4fea23da8576..be3d198043ff9 100644
@@ -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 */

mm/damon/core.c +8 -0

index c1d1091d307e4..37454e8c9c510 100644
@@ -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

Signature · 1 line

2.47.3

Andrew Morton

maintainer

2026-03-20 02:48 UTC #1 lore raw

Quoted 20 lines · On Thu, 19 Mar 2026 07:52:17 -0700 SeongJae Park <[email protected]> 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.

Quoted 10 lines

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

SeongJae Park

author maintainer

2026-03-20 03:15 UTC #2 lore raw

Quoted 33 lines · On Thu, 19 Mar 2026 19:48:49 -0700 Andrew Morton <[email protected]>…

Quoted 20 lines · On Thu, 19 Mar 2026 07:52:17 -0700 SeongJae Park <[email protected]> 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.
Quoted 10 lines
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.

Quoted 2 lines

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 <[email protected]>
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 <[email protected]>

---
 mm/damon/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/damon/core.c +1 -1

index 37454e8c9c510..3e1890d64d067 100644
@@ -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;
 	}

Signature · 1 line

2.47.3

SeongJae Park

author maintainer

2026-03-21 02:16 UTC #3 lore raw

In reply to Andrew Morton

Quoted 33 lines · On Thu, 19 Mar 2026 19:48:49 -0700 Andrew Morton <[email protected]>…

Quoted 20 lines · On Thu, 19 Mar 2026 07:52:17 -0700 SeongJae Park <[email protected]> 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.
Quoted 10 lines
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.

Quoted 2 lines

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

SeongJae Park

author maintainer

2026-03-21 02:25 UTC #4 lore raw

Quoted 67 lines · On Fri, 20 Mar 2026 19:16:27 -0700 SeongJae Park <[email protected]> wrote:

Quoted 33 lines · On Thu, 19 Mar 2026 19:48:49 -0700 Andrew Morton <[email protected]>…
Quoted 20 lines · On Thu, 19 Mar 2026 07:52:17 -0700 SeongJae Park <[email protected]> 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.
Quoted 10 lines
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.
Quoted 2 lines
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.)

Quoted 2 lines

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

[...]

SeongJae Park

author maintainer

2026-03-22 19:26 UTC #5 lore raw

In reply to SeongJae Park

On Fri, 20 Mar 2026 19:16:35 SeongJae Park <[email protected]> wrote:
[...]

Quoted 29 lines

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.

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

Quoted 2 lines

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

[...]

SeongJae Park

author maintainer

2026-03-22 19:32 UTC #6 lore raw

On Sun, 22 Mar 2026 12:26:48 SeongJae Park <[email protected]> wrote:
[...]

Quoted 5 lines

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

[...]