DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH 6.1.y] mm/damon/core: fix unconditionally skip last region
       [not found] <2026092949-surpass-thimble-02a8@gregkh>
@ 2026-09-30  9:48 ` SJ Park
  2026-09-30 10:05   ` sashiko-bot
  2026-10-02 14:19   ` Sasha Levin
  2026-10-09 14:48 ` [PATCH 6.1.y v2 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
  2026-10-10  6:30 ` [PATCH 6.1.y v3 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
  2 siblings, 2 replies; 18+ messages in thread
From: SJ Park @ 2026-09-30  9:48 UTC (permalink / raw)
  To: stable; +Cc: damon, Liew Rui Yan, SJ Park, Andrew Morton

From: Liew Rui Yan <aethernet65535@gmail.com>

Once quota set, the charge_{target,addr}_from unconditionally skips and
resets at the last region of the tracked target, so the last region can be
skipped even when it has not been processed.

Example:

    1. Target has 2 regions: R1 (0-100 bytes) and R2 (100-200 bytes).
    2. Quota is configured to process only 100 bytes per window.
    3. Window 1: Processes R1 (0-100).  Quota is full.  charge_{target,
       addr}_from is saved at (Target, 100).
    4. Window 2: The loop reaches R2.  Because R2 is
       damon_last_region(t), the old code unconditionally returns true,
       skipping R2 entirely and resetting the charge_{target,addr}_from.

    Result: R2 is permanently skipped even though it has never been
    processed.

However, it is important to note that this is a very minor issue.  This is
because it is triggered only when the previous window saved/kept
charge_{target,addr}_from, and in the next window, all regions except the
last region were skipped by damos_skip_charged_region().

Fix this by only resetting the charge_{target,addr}_from when last region
is reached, only skipping when it is applied or cannot split.

Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
Fixes: 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: SJ Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Cc: <stable@vger.kernel.org> # v5.16.x
(cherry picked from commit b3723b596b548c837a766aae3553c14a7b15af2b)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 859a6a636ab0e..64ff03b15fdef 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -756,6 +756,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		unsigned long sz = damon_sz_region(r);
 		struct timespec64 begin, end;
 		unsigned long sz_applied = 0;
+		bool skip = false;
 
 		if (!s->wmarks.activated)
 			continue;
@@ -768,14 +769,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		if (quota->charge_target_from) {
 			if (t != quota->charge_target_from)
 				continue;
-			if (r == damon_last_region(t)) {
-				quota->charge_target_from = NULL;
-				quota->charge_addr_from = 0;
-				continue;
-			}
 			if (quota->charge_addr_from &&
-					r->ar.end <= quota->charge_addr_from)
-				continue;
+					r->ar.end <= quota->charge_addr_from) {
+				skip = true;
+				goto clean_charge_from_skip;
+			}
 
 			if (quota->charge_addr_from && r->ar.start <
 					quota->charge_addr_from) {
@@ -783,17 +781,25 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 						r->ar.start, DAMON_MIN_REGION);
 				if (!sz) {
 					if (damon_sz_region(r) <=
-					    DAMON_MIN_REGION)
-						continue;
+					    DAMON_MIN_REGION) {
+						skip = true;
+						goto clean_charge_from_skip;
+					}
 					sz = DAMON_MIN_REGION;
 				}
 				damon_split_region_at(t, r, sz);
 				r = damon_next_region(r);
 				sz = damon_sz_region(r);
+				skip = false;
 			}
+		}
+clean_charge_from_skip:
+		if (r == damon_last_region(t)) {
 			quota->charge_target_from = NULL;
 			quota->charge_addr_from = 0;
 		}
+		if (skip)
+			continue;
 
 		if (!damos_valid_target(c, t, r, s))
 			continue;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y] mm/damon/core: fix unconditionally skip last region
  2026-09-30  9:48 ` [PATCH 6.1.y] mm/damon/core: fix unconditionally skip last region SJ Park
@ 2026-09-30 10:05   ` sashiko-bot
  2026-09-30 10:23     ` SJ Park
  2026-10-02 14:19   ` Sasha Levin
  1 sibling, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-09-30 10:05 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/damon: out-of-bounds memory access when region split fails on last region
--

commit e8c0d519ccfab8bb3783274c1139b24ab75f8aa1
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon/core: fix unconditionally skip last region

This commit fixes an issue where the last region of a target is
unconditionally skipped and reset when a quota is set, even if it hasn't
been processed.

Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 859a6a636ab0e..64ff03b15fdef 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -768,14 +769,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
>  		if (quota->charge_target_from) {
>  			if (t != quota->charge_target_from)
>  				continue;
> -			if (r == damon_last_region(t)) {
> -				quota->charge_target_from = NULL;
> -				quota->charge_addr_from = 0;
> -				continue;
> -			}

[ ... ]

> @@ -783,17 +781,25 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
>  						r->ar.start, DAMON_MIN_REGION);
>  				if (!sz) {
>  					if (damon_sz_region(r) <=
> -					    DAMON_MIN_REGION)
> -						continue;
> +					    DAMON_MIN_REGION) {
> +						skip = true;
> +						goto clean_charge_from_skip;
> +					}
>  					sz = DAMON_MIN_REGION;
>  				}
>  				damon_split_region_at(t, r, sz);
>  				r = damon_next_region(r);
>  				sz = damon_sz_region(r);

[Severity: High]
Since this patch removes the early check for the last region, what happens
in damon_do_apply_schemes() if damon_split_region_at() fails on the last
region?

If damon_split_region_at() fails under memory pressure, it returns silently
without splitting the region. The code then unconditionally calls
r = damon_next_region(r).

Because r was the last region and was not split, its list.next points to the
list head. damon_next_region() then returns an invalid pointer. 

The next line calls damon_sz_region(r), which dereferences this invalid
pointer and causes an out-of-bounds memory access.

Does this code need a check to ensure the region was successfully split before
advancing the pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930094853.52736-1-sj@kernel.org?part=1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y] mm/damon/core: fix unconditionally skip last region
  2026-09-30 10:05   ` sashiko-bot
@ 2026-09-30 10:23     ` SJ Park
  0 siblings, 0 replies; 18+ messages in thread
From: SJ Park @ 2026-09-30 10:23 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Wed, 30 Sep 2026 10:05:10 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm/damon: out-of-bounds memory access when region split fails on last region
> --
> 
> commit e8c0d519ccfab8bb3783274c1139b24ab75f8aa1
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: fix unconditionally skip last region
> 
> This commit fixes an issue where the last region of a target is
> unconditionally skipped and reset when a quota is set, even if it hasn't
> been processed.
> 
> Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 859a6a636ab0e..64ff03b15fdef 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -768,14 +769,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
> >  		if (quota->charge_target_from) {
> >  			if (t != quota->charge_target_from)
> >  				continue;
> > -			if (r == damon_last_region(t)) {
> > -				quota->charge_target_from = NULL;
> > -				quota->charge_addr_from = 0;
> > -				continue;
> > -			}
> 
> [ ... ]
> 
> > @@ -783,17 +781,25 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
> >  						r->ar.start, DAMON_MIN_REGION);
> >  				if (!sz) {
> >  					if (damon_sz_region(r) <=
> > -					    DAMON_MIN_REGION)
> > -						continue;
> > +					    DAMON_MIN_REGION) {
> > +						skip = true;
> > +						goto clean_charge_from_skip;
> > +					}
> >  					sz = DAMON_MIN_REGION;
> >  				}
> >  				damon_split_region_at(t, r, sz);
> >  				r = damon_next_region(r);
> >  				sz = damon_sz_region(r);
> 
> [Severity: High]
> Since this patch removes the early check for the last region, what happens
> in damon_do_apply_schemes() if damon_split_region_at() fails on the last
> region?
> 
> If damon_split_region_at() fails under memory pressure, it returns silently
> without splitting the region. The code then unconditionally calls
> r = damon_next_region(r).
> 
> Because r was the last region and was not split, its list.next points to the
> list head. damon_next_region() then returns an invalid pointer. 
> 
> The next line calls damon_sz_region(r), which dereferences this invalid
> pointer and causes an out-of-bounds memory access.
> 
> Does this code need a check to ensure the region was successfully split before
> advancing the pointer?

The out-of-bounds memory access is indeed occurs due to this patch.  But the
lack of damon_split_region_at() error handling was anyway a bug.  So, in a
sense, pre-existing issue.  I will work on fixing it separately.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260930094853.52736-1-sj@kernel.org?part=1


Thanks,
SJ

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y] mm/damon/core: fix unconditionally skip last region
  2026-09-30  9:48 ` [PATCH 6.1.y] mm/damon/core: fix unconditionally skip last region SJ Park
  2026-09-30 10:05   ` sashiko-bot
@ 2026-10-02 14:19   ` Sasha Levin
  1 sibling, 0 replies; 18+ messages in thread
From: Sasha Levin @ 2026-10-02 14:19 UTC (permalink / raw)
  To: stable; +Cc: Sasha Levin, damon, Liew Rui Yan, SJ Park, Andrew Morton

> Fix this by only resetting the charge_{target,addr}_from when last region
> is reached, only skipping when it is applied or cannot split.

I'll hold this until the damon_split_region_at() failure handling fix
you mentioned is posted, and then take 6.12, 6.6 and 6.1 together with
it.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 6.1.y v2 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions()
       [not found] <2026092949-surpass-thimble-02a8@gregkh>
  2026-09-30  9:48 ` [PATCH 6.1.y] mm/damon/core: fix unconditionally skip last region SJ Park
@ 2026-10-09 14:48 ` SJ Park
  2026-10-09 14:48   ` [PATCH 6.1.y v2 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
  2026-10-09 14:48   ` [PATCH 6.1.y v2 2/2] mm/damon/core: fix unconditionally skip last region SJ Park
  2026-10-10  6:30 ` [PATCH 6.1.y v3 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
  2 siblings, 2 replies; 18+ messages in thread
From: SJ Park @ 2026-10-09 14:48 UTC (permalink / raw)
  To: stable; +Cc: damon, SJ Park

Patch 1 is a dependency of patch 2.  Without it, patch 2 introduces
out-of-bounds memory access bug that was found by Sashiko.  Patch 2
fixes a bug that categorized to be backported to stable@.

Changes from v1
- v1: https://lore.kernel.org/20260930094853.52736-1-sj@kernel.org
- Fix out-of-bounds memory access bug by adding patch 1.

Liew Rui Yan (1):
  mm/damon/core: fix unconditionally skip last region

SeongJae Park (1):
  mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

 mm/damon/core.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)


base-commit: af13aea0793685297faa77d75b825f0e5a99319c
-- 
2.47.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 6.1.y v2 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
  2026-10-09 14:48 ` [PATCH 6.1.y v2 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
@ 2026-10-09 14:48   ` SJ Park
  2026-10-09 14:56     ` sashiko-bot
  2026-10-09 14:48   ` [PATCH 6.1.y v2 2/2] mm/damon/core: fix unconditionally skip last region SJ Park
  1 sibling, 1 reply; 18+ messages in thread
From: SJ Park @ 2026-10-09 14:48 UTC (permalink / raw)
  To: stable; +Cc: damon, SeongJae Park, Andrew Morton

From: SeongJae Park <sj@kernel.org>

kdamond_apply_schemes() is using damon_for_each_region_safe(), which is
safe for deallocation of the region inside the loop.  However, the loop
internal logic does not deallocate regions.  Hence it is only wasting the
next pointer.  Also, it causes a problem.

When an address filter is applied, and there is a region that intersects
with the filter, the filter splits the region on the filter boundary.  The
intention is to let DAMOS apply action to only filtered-in address ranges.
However, it is using damon_for_each_region_safe(), which sets the next
region before the execution of the iteration.  Hence, the region that
split and now will be next to the previous region, is simply ignored.  As
a result, DAMOS applies the action to target regions bit slower than
expected, when the address filter is used.  Shouldn't be a big problem but
definitely better to be fixed.  damos_skip_charged_region() was working
around the issue using a double pointer hack.

Use damon_for_each_region(), which is safe for this use case.  And drop
the work around in damos_skip_charged_region().

Link: https://lkml.kernel.org/r/20260227170623.95384-3-sj@kernel.org
Signed-off-by: SeongJae Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 1745ccbd2907db2bdaa843e4abccde4fdaccbe5d)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 7afbc20833ea..c65d2cbeea6b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -858,7 +858,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
 static void kdamond_apply_schemes(struct damon_ctx *c)
 {
 	struct damon_target *t;
-	struct damon_region *r, *next_r;
+	struct damon_region *r;
 	struct damos *s;
 
 	damon_for_each_scheme(s, c) {
@@ -912,7 +912,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 	}
 
 	damon_for_each_target(t, c) {
-		damon_for_each_region_safe(r, next_r, t)
+		damon_for_each_region(r, t)
 			damon_do_apply_schemes(c, t, r);
 	}
 }
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6.1.y v2 2/2] mm/damon/core: fix unconditionally skip last region
  2026-10-09 14:48 ` [PATCH 6.1.y v2 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
  2026-10-09 14:48   ` [PATCH 6.1.y v2 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
@ 2026-10-09 14:48   ` SJ Park
  2026-10-09 15:04     ` sashiko-bot
  1 sibling, 1 reply; 18+ messages in thread
From: SJ Park @ 2026-10-09 14:48 UTC (permalink / raw)
  To: stable; +Cc: damon, Liew Rui Yan, SJ Park, Andrew Morton

From: Liew Rui Yan <aethernet65535@gmail.com>

Once quota set, the charge_{target,addr}_from unconditionally skips and
resets at the last region of the tracked target, so the last region can be
skipped even when it has not been processed.

Example:

    1. Target has 2 regions: R1 (0-100 bytes) and R2 (100-200 bytes).
    2. Quota is configured to process only 100 bytes per window.
    3. Window 1: Processes R1 (0-100).  Quota is full.  charge_{target,
       addr}_from is saved at (Target, 100).
    4. Window 2: The loop reaches R2.  Because R2 is
       damon_last_region(t), the old code unconditionally returns true,
       skipping R2 entirely and resetting the charge_{target,addr}_from.

    Result: R2 is permanently skipped even though it has never been
    processed.

However, it is important to note that this is a very minor issue.  This is
because it is triggered only when the previous window saved/kept
charge_{target,addr}_from, and in the next window, all regions except the
last region were skipped by damos_skip_charged_region().

Fix this by only resetting the charge_{target,addr}_from when last region
is reached, only skipping when it is applied or cannot split.

Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
Fixes: 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: SJ Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Cc: <stable@vger.kernel.org> # v5.16.x
(cherry picked from commit b3723b596b548c837a766aae3553c14a7b15af2b)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index c65d2cbeea6b..f2f6f125c09b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -756,6 +756,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		unsigned long sz = damon_sz_region(r);
 		struct timespec64 begin, end;
 		unsigned long sz_applied = 0;
+		bool skip = false;
 
 		if (!s->wmarks.activated)
 			continue;
@@ -768,14 +769,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		if (quota->charge_target_from) {
 			if (t != quota->charge_target_from)
 				continue;
-			if (r == damon_last_region(t)) {
-				quota->charge_target_from = NULL;
-				quota->charge_addr_from = 0;
-				continue;
-			}
 			if (quota->charge_addr_from &&
-					r->ar.end <= quota->charge_addr_from)
-				continue;
+					r->ar.end <= quota->charge_addr_from) {
+				skip = true;
+				goto clean_charge_from_skip;
+			}
 
 			if (quota->charge_addr_from && r->ar.start <
 					quota->charge_addr_from) {
@@ -783,17 +781,25 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 						r->ar.start, DAMON_MIN_REGION);
 				if (!sz) {
 					if (damon_sz_region(r) <=
-					    DAMON_MIN_REGION)
-						continue;
+					    DAMON_MIN_REGION) {
+						skip = true;
+						goto clean_charge_from_skip;
+					}
 					sz = DAMON_MIN_REGION;
 				}
 				damon_split_region_at(t, r, sz);
 				r = damon_next_region(r);
 				sz = damon_sz_region(r);
+				skip = false;
 			}
+		}
+clean_charge_from_skip:
+		if (r == damon_last_region(t)) {
 			quota->charge_target_from = NULL;
 			quota->charge_addr_from = 0;
 		}
+		if (skip)
+			continue;
 
 		if (!damos_valid_target(c, t, r, s))
 			continue;
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y v2 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
  2026-10-09 14:48   ` [PATCH 6.1.y v2 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
@ 2026-10-09 14:56     ` sashiko-bot
  2026-10-09 15:09       ` SJ Park
  0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-10-09 14:56 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/damon: double processing and skipped regions due to broken backport
--

commit 18797502eb0b5cf3a96dbf53e3e238e6ea4158ce
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

This commit replaces the damon_for_each_region_safe iterator with
damon_for_each_region in kdamond_apply_schemes. This change ensures that
newly split regions from address filters are properly processed during the
iteration.

Link: https://lkml.kernel.org/r/20260227170623.95384-3-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 7afbc20833ea4..c65d2cbeea6b5 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -912,7 +912,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
>  	}
>  
>  	damon_for_each_target(t, c) {
> -		damon_for_each_region_safe(r, next_r, t)
> +		damon_for_each_region(r, t)

[Severity: High]
Does this change result in regions being skipped and double-processed?

The commit message mentions dropping the workaround in
damos_skip_charged_region(), but in this stable kernel version, that helper
does not exist yet and the workaround is still inlined in
damon_do_apply_schemes(). Since the inline workaround wasn't removed in this
backport, it appears to conflict with the new loop iterator.

If a scheme's quota->charge_addr_from check triggers a region split in
damon_do_apply_schemes(), the local pointer is advanced:

mm/damon/core.c:damon_do_apply_schemes() {
    ...
			if (quota->charge_addr_from && r->ar.start <
					quota->charge_addr_from) {
				sz = ALIGN_DOWN(quota->charge_addr_from -
						r->ar.start, DAMON_MIN_REGION);
				if (!sz) {
					if (damon_sz_region(r) <=
					    DAMON_MIN_REGION)
						continue;
					sz = DAMON_MIN_REGION;
				}
				damon_split_region_at(t, r, sz);
				r = damon_next_region(r);
				sz = damon_sz_region(r);
			}
    ...
}

Because r is reassigned locally within the inner scheme loop, subsequent
schemes will apply their actions to the newly created second-half region,
completely skipping the first half. 

Then, when damon_do_apply_schemes() returns to the outer loop in
kdamond_apply_schemes():

		damon_for_each_region(r, t)
			damon_do_apply_schemes(c, t, r);

The iterator will advance naturally to the newly split second-half region.
This causes damon_do_apply_schemes() to be called a second time on the exact
same region, leading to DAMON actions being applied multiple times to the
same memory, double-charged quotas, and corrupted statistics.

>  			damon_do_apply_schemes(c, t, r);
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009144808.18317-1-sj@kernel.org?part=1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y v2 2/2] mm/damon/core: fix unconditionally skip last region
  2026-10-09 14:48   ` [PATCH 6.1.y v2 2/2] mm/damon/core: fix unconditionally skip last region SJ Park
@ 2026-10-09 15:04     ` sashiko-bot
  2026-10-09 15:11       ` SJ Park
  0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-10-09 15:04 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/damon: Out-of-bounds access due to unchecked split failure on last region
--

commit aacf3c387b1624c2944e2e5044dad18623db3d5f
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon/core: fix unconditionally skip last region

This commit modifies the quota mechanism in DAMON to only reset the charge
target and address from when the last region is reached. It ensures the last
region is not skipped if it has not been processed.

Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index c65d2cbeea6b5..f2f6f125c09b1 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -768,14 +769,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
>  		if (quota->charge_target_from) {
>  			if (t != quota->charge_target_from)
>  				continue;
> -			if (r == damon_last_region(t)) {
> -				quota->charge_target_from = NULL;
> -				quota->charge_addr_from = 0;
> -				continue;
> -			}
>  			if (quota->charge_addr_from &&
> -					r->ar.end <= quota->charge_addr_from)
> -				continue;
> +					r->ar.end <= quota->charge_addr_from) {
> +				skip = true;
> +				goto clean_charge_from_skip;
> +			}
>  
>  			if (quota->charge_addr_from && r->ar.start <
>  					quota->charge_addr_from) {
> @@ -783,17 +781,25 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
>  						r->ar.start, DAMON_MIN_REGION);
>  				if (!sz) {
>  					if (damon_sz_region(r) <=
> -					    DAMON_MIN_REGION)
> -						continue;
> +					    DAMON_MIN_REGION) {
> +						skip = true;
> +						goto clean_charge_from_skip;
> +					}
>  					sz = DAMON_MIN_REGION;
>  				}
>  				damon_split_region_at(t, r, sz);
>  				r = damon_next_region(r);
>  				sz = damon_sz_region(r);

[Severity: High]
Since this commit removes the check that bypassed damon_split_region_at() for
the target's last region, does this introduce an out-of-bounds access if
memory allocation fails?

If the last region overlaps the charge boundary, it is now passed to
damon_split_region_at(). This function performs a non-blocking memory
allocation that can fail under memory pressure and return silently without
splitting the region:

mm/damon/core.c:damon_split_region_at() {
    new = damon_new_region(r->ar.start + sz_r, r->ar.end);
    if (!new)
        return;
    ...
}

If the allocation fails when processing the last region, the region is not
split. The code then unconditionally executes:

    r = damon_next_region(r);
    sz = damon_sz_region(r);

Because r was the last region and wasn't split, damon_next_region() reads
the list.next pointer which points back to the list head:

include/linux/damon.h:damon_next_region() {
    return container_of(r->list.next, struct damon_region, list);
}

This produces an out-of-bounds pointer derived from the list head. When this
pointer is immediately dereferenced by damon_sz_region(r), could this
regression result in out-of-bounds memory reads?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009144808.18317-1-sj@kernel.org?part=2

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y v2 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
  2026-10-09 14:56     ` sashiko-bot
@ 2026-10-09 15:09       ` SJ Park
  0 siblings, 0 replies; 18+ messages in thread
From: SJ Park @ 2026-10-09 15:09 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Fri, 09 Oct 2026 14:56:56 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm/damon: double processing and skipped regions due to broken backport
> --
> 
> commit 18797502eb0b5cf3a96dbf53e3e238e6ea4158ce
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
> 
> This commit replaces the damon_for_each_region_safe iterator with
> damon_for_each_region in kdamond_apply_schemes. This change ensures that
> newly split regions from address filters are properly processed during the
> iteration.
> 
> Link: https://lkml.kernel.org/r/20260227170623.95384-3-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 7afbc20833ea4..c65d2cbeea6b5 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -912,7 +912,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
> >  	}
> >  
> >  	damon_for_each_target(t, c) {
> > -		damon_for_each_region_safe(r, next_r, t)
> > +		damon_for_each_region(r, t)
> 
> [Severity: High]
> Does this change result in regions being skipped and double-processed?
> 
> The commit message mentions dropping the workaround in
> damos_skip_charged_region(), but in this stable kernel version, that helper
> does not exist yet and the workaround is still inlined in
> damon_do_apply_schemes(). Since the inline workaround wasn't removed in this
> backport, it appears to conflict with the new loop iterator.

Good catch.  I will fix this in the next spin.

> 
> If a scheme's quota->charge_addr_from check triggers a region split in
> damon_do_apply_schemes(), the local pointer is advanced:
> 
> mm/damon/core.c:damon_do_apply_schemes() {
>     ...
> 			if (quota->charge_addr_from && r->ar.start <
> 					quota->charge_addr_from) {
> 				sz = ALIGN_DOWN(quota->charge_addr_from -
> 						r->ar.start, DAMON_MIN_REGION);
> 				if (!sz) {
> 					if (damon_sz_region(r) <=
> 					    DAMON_MIN_REGION)
> 						continue;
> 					sz = DAMON_MIN_REGION;
> 				}
> 				damon_split_region_at(t, r, sz);
> 				r = damon_next_region(r);
> 				sz = damon_sz_region(r);
> 			}
>     ...
> }
> 
> Because r is reassigned locally within the inner scheme loop, subsequent
> schemes will apply their actions to the newly created second-half region,
> completely skipping the first half. 
> 
> Then, when damon_do_apply_schemes() returns to the outer loop in
> kdamond_apply_schemes():
> 
> 		damon_for_each_region(r, t)
> 			damon_do_apply_schemes(c, t, r);
> 
> The iterator will advance naturally to the newly split second-half region.
> This causes damon_do_apply_schemes() to be called a second time on the exact
> same region, leading to DAMON actions being applied multiple times to the
> same memory, double-charged quotas, and corrupted statistics.

This is an issue that exist in the mainline, too.  I will separately work on
it.

> 
> >  			damon_do_apply_schemes(c, t, r);
> >  	}
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261009144808.18317-1-sj@kernel.org?part=1


Thanks,
SJ

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y v2 2/2] mm/damon/core: fix unconditionally skip last region
  2026-10-09 15:04     ` sashiko-bot
@ 2026-10-09 15:11       ` SJ Park
  0 siblings, 0 replies; 18+ messages in thread
From: SJ Park @ 2026-10-09 15:11 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Fri, 09 Oct 2026 15:04:56 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm/damon: Out-of-bounds access due to unchecked split failure on last region
> --
> 
> commit aacf3c387b1624c2944e2e5044dad18623db3d5f
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: fix unconditionally skip last region
> 
> This commit modifies the quota mechanism in DAMON to only reset the charge
> target and address from when the last region is reached. It ensures the last
> region is not skipped if it has not been processed.
> 
> Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index c65d2cbeea6b5..f2f6f125c09b1 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -768,14 +769,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
> >  		if (quota->charge_target_from) {
> >  			if (t != quota->charge_target_from)
> >  				continue;
> > -			if (r == damon_last_region(t)) {
> > -				quota->charge_target_from = NULL;
> > -				quota->charge_addr_from = 0;
> > -				continue;
> > -			}
> >  			if (quota->charge_addr_from &&
> > -					r->ar.end <= quota->charge_addr_from)
> > -				continue;
> > +					r->ar.end <= quota->charge_addr_from) {
> > +				skip = true;
> > +				goto clean_charge_from_skip;
> > +			}
> >  
> >  			if (quota->charge_addr_from && r->ar.start <
> >  					quota->charge_addr_from) {
> > @@ -783,17 +781,25 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
> >  						r->ar.start, DAMON_MIN_REGION);
> >  				if (!sz) {
> >  					if (damon_sz_region(r) <=
> > -					    DAMON_MIN_REGION)
> > -						continue;
> > +					    DAMON_MIN_REGION) {
> > +						skip = true;
> > +						goto clean_charge_from_skip;
> > +					}
> >  					sz = DAMON_MIN_REGION;
> >  				}
> >  				damon_split_region_at(t, r, sz);
> >  				r = damon_next_region(r);
> >  				sz = damon_sz_region(r);
> 
> [Severity: High]
> Since this commit removes the check that bypassed damon_split_region_at() for
> the target's last region, does this introduce an out-of-bounds access if
> memory allocation fails?
> 
> If the last region overlaps the charge boundary, it is now passed to
> damon_split_region_at(). This function performs a non-blocking memory
> allocation that can fail under memory pressure and return silently without
> splitting the region:
> 
> mm/damon/core.c:damon_split_region_at() {
>     new = damon_new_region(r->ar.start + sz_r, r->ar.end);
>     if (!new)
>         return;
>     ...
> }
> 
> If the allocation fails when processing the last region, the region is not
> split. The code then unconditionally executes:
> 
>     r = damon_next_region(r);
>     sz = damon_sz_region(r);
> 
> Because r was the last region and wasn't split, damon_next_region() reads
> the list.next pointer which points back to the list head:
> 
> include/linux/damon.h:damon_next_region() {
>     return container_of(r->list.next, struct damon_region, list);
> }
> 
> This produces an out-of-bounds pointer derived from the list head. When this
> pointer is immediately dereferenced by damon_sz_region(r), could this
> regression result in out-of-bounds memory reads?

Good finding.  Patch 1 should avoided this issue by removing the safe iterator
workaround, but I mistakenly didn't remove the workaround.  I will fix it in
the next spin.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261009144808.18317-1-sj@kernel.org?part=2


Thanks,
SJ

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 6.1.y v3 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions()
       [not found] <2026092949-surpass-thimble-02a8@gregkh>
  2026-09-30  9:48 ` [PATCH 6.1.y] mm/damon/core: fix unconditionally skip last region SJ Park
  2026-10-09 14:48 ` [PATCH 6.1.y v2 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
@ 2026-10-10  6:30 ` SJ Park
  2026-10-10  6:30   ` [PATCH 6.1.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
  2026-10-10  6:30   ` [PATCH 6.1.y v3 2/2] mm/damon/core: fix unconditionally skip last region SJ Park
  2 siblings, 2 replies; 18+ messages in thread
From: SJ Park @ 2026-10-10  6:30 UTC (permalink / raw)
  To: stable; +Cc: damon, SJ Park

Patch 1 is a dependency of patch 2.  Without it, patch 2 introduces
out-of-bounds memory access bug that was found by Sashiko.  Patch 2
fixes a bug that categorized to be backported to stable@.

Changes from v2
- v2: https://lore.kernel.org/20261009144808.18317-1-sj@kernel.org
- Drop the safe region traversal workaround in patch 1.
Changes from v1
- v1: https://lore.kernel.org/20260930094853.52736-1-sj@kernel.org
- Fix out-of-bounds memory access bug by adding patch 1.

Liew Rui Yan (1):
  mm/damon/core: fix unconditionally skip last region

SeongJae Park (1):
  mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

 mm/damon/core.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)


base-commit: 5f9615e6a3b5f768df8b69ab622454fcb3af0a34
-- 
2.47.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 6.1.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
  2026-10-10  6:30 ` [PATCH 6.1.y v3 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
@ 2026-10-10  6:30   ` SJ Park
  2026-10-10  6:41     ` sashiko-bot
  2026-10-10  6:30   ` [PATCH 6.1.y v3 2/2] mm/damon/core: fix unconditionally skip last region SJ Park
  1 sibling, 1 reply; 18+ messages in thread
From: SJ Park @ 2026-10-10  6:30 UTC (permalink / raw)
  To: stable; +Cc: damon, SeongJae Park, Andrew Morton

From: SeongJae Park <sj@kernel.org>

kdamond_apply_schemes() is using damon_for_each_region_safe(), which is
safe for deallocation of the region inside the loop.  However, the loop
internal logic does not deallocate regions.  Hence it is only wasting the
next pointer.  Also, it causes a problem.

When an address filter is applied, and there is a region that intersects
with the filter, the filter splits the region on the filter boundary.  The
intention is to let DAMOS apply action to only filtered-in address ranges.
However, it is using damon_for_each_region_safe(), which sets the next
region before the execution of the iteration.  Hence, the region that
split and now will be next to the previous region, is simply ignored.  As
a result, DAMOS applies the action to target regions bit slower than
expected, when the address filter is used.  Shouldn't be a big problem but
definitely better to be fixed.  damos_skip_charged_region() was working
around the issue using a double pointer hack.

Use damon_for_each_region(), which is safe for this use case.  And drop
the work around in damos_skip_charged_region().

Link: https://lkml.kernel.org/r/20260227170623.95384-3-sj@kernel.org
Signed-off-by: SeongJae Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 1745ccbd2907db2bdaa843e4abccde4fdaccbe5d)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 7afbc20833ea..b5c85e648859 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -788,8 +788,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 					sz = DAMON_MIN_REGION;
 				}
 				damon_split_region_at(t, r, sz);
-				r = damon_next_region(r);
-				sz = damon_sz_region(r);
+				continue;
 			}
 			quota->charge_target_from = NULL;
 			quota->charge_addr_from = 0;
@@ -858,7 +857,7 @@ static void damos_set_effective_quota(struct damos_quota *quota)
 static void kdamond_apply_schemes(struct damon_ctx *c)
 {
 	struct damon_target *t;
-	struct damon_region *r, *next_r;
+	struct damon_region *r;
 	struct damos *s;
 
 	damon_for_each_scheme(s, c) {
@@ -912,7 +911,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 	}
 
 	damon_for_each_target(t, c) {
-		damon_for_each_region_safe(r, next_r, t)
+		damon_for_each_region(r, t)
 			damon_do_apply_schemes(c, t, r);
 	}
 }
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6.1.y v3 2/2] mm/damon/core: fix unconditionally skip last region
  2026-10-10  6:30 ` [PATCH 6.1.y v3 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
  2026-10-10  6:30   ` [PATCH 6.1.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
@ 2026-10-10  6:30   ` SJ Park
  2026-10-10  6:46     ` sashiko-bot
  1 sibling, 1 reply; 18+ messages in thread
From: SJ Park @ 2026-10-10  6:30 UTC (permalink / raw)
  To: stable; +Cc: damon, Liew Rui Yan, SJ Park, Andrew Morton

From: Liew Rui Yan <aethernet65535@gmail.com>

Once quota set, the charge_{target,addr}_from unconditionally skips and
resets at the last region of the tracked target, so the last region can be
skipped even when it has not been processed.

Example:

    1. Target has 2 regions: R1 (0-100 bytes) and R2 (100-200 bytes).
    2. Quota is configured to process only 100 bytes per window.
    3. Window 1: Processes R1 (0-100).  Quota is full.  charge_{target,
       addr}_from is saved at (Target, 100).
    4. Window 2: The loop reaches R2.  Because R2 is
       damon_last_region(t), the old code unconditionally returns true,
       skipping R2 entirely and resetting the charge_{target,addr}_from.

    Result: R2 is permanently skipped even though it has never been
    processed.

However, it is important to note that this is a very minor issue.  This is
because it is triggered only when the previous window saved/kept
charge_{target,addr}_from, and in the next window, all regions except the
last region were skipped by damos_skip_charged_region().

Fix this by only resetting the charge_{target,addr}_from when last region
is reached, only skipping when it is applied or cannot split.

Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
Fixes: 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: SJ Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Cc: <stable@vger.kernel.org> # v5.16.x
(cherry picked from commit b3723b596b548c837a766aae3553c14a7b15af2b)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index b5c85e648859..1b9fb00405cf 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -756,6 +756,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		unsigned long sz = damon_sz_region(r);
 		struct timespec64 begin, end;
 		unsigned long sz_applied = 0;
+		bool skip = false;
 
 		if (!s->wmarks.activated)
 			continue;
@@ -768,14 +769,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		if (quota->charge_target_from) {
 			if (t != quota->charge_target_from)
 				continue;
-			if (r == damon_last_region(t)) {
-				quota->charge_target_from = NULL;
-				quota->charge_addr_from = 0;
-				continue;
-			}
 			if (quota->charge_addr_from &&
-					r->ar.end <= quota->charge_addr_from)
-				continue;
+					r->ar.end <= quota->charge_addr_from) {
+				skip = true;
+				goto clean_charge_from_skip;
+			}
 
 			if (quota->charge_addr_from && r->ar.start <
 					quota->charge_addr_from) {
@@ -783,16 +781,23 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 						r->ar.start, DAMON_MIN_REGION);
 				if (!sz) {
 					if (damon_sz_region(r) <=
-					    DAMON_MIN_REGION)
-						continue;
+					    DAMON_MIN_REGION) {
+						skip = true;
+						goto clean_charge_from_skip;
+					}
 					sz = DAMON_MIN_REGION;
 				}
 				damon_split_region_at(t, r, sz);
-				continue;
+				skip = true;
 			}
+		}
+clean_charge_from_skip:
+		if (r == damon_last_region(t)) {
 			quota->charge_target_from = NULL;
 			quota->charge_addr_from = 0;
 		}
+		if (skip)
+			continue;
 
 		if (!damos_valid_target(c, t, r, s))
 			continue;
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
  2026-10-10  6:30   ` [PATCH 6.1.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
@ 2026-10-10  6:41     ` sashiko-bot
  2026-10-10  7:11       ` SJ Park
  0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-10-10  6:41 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/damon: double-application of earlier schemes to newly split regions
--

commit 01d768c4d318527360ef5913771d20f22a99984a
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

kdamond_apply_schemes() is using damon_for_each_region_safe(), which is
safe for deallocation of the region inside the loop. However, the loop
internal logic does not deallocate regions. Hence it is only wasting the
next pointer.

Link: https://lkml.kernel.org/r/20260227170623.95384-3-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 7afbc20833ea4..b5c85e6488596 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -912,7 +911,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
>  	}
>  
>  	damon_for_each_target(t, c) {
> -		damon_for_each_region_safe(r, next_r, t)
> +		damon_for_each_region(r, t)
>  			damon_do_apply_schemes(c, t, r);

[Severity: High]
Does this code cause earlier schemes to be applied twice to newly split
regions?

If multiple schemes are configured, an earlier scheme (e.g. Scheme 0) might
apply its action to a full, un-split region. If a later scheme (e.g.
Scheme 1) then decides to split the region, damon_split_region_at() inserts
the newly split part into the list immediately after the current region.

Because this loop in kdamond_apply_schemes() now uses
damon_for_each_region() instead of the safe variant, the outer loop will
advance to this newly split region and call damon_do_apply_schemes() on it
again.

Will this cause damon_do_apply_schemes() to evaluate the new region from
the beginning of the scheme list, resulting in Scheme 0 executing its
memory management actions (like madvise) and counting statistics a second
time on the same memory range?

>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010063042.8611-1-sj@kernel.org?part=1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y v3 2/2] mm/damon/core: fix unconditionally skip last region
  2026-10-10  6:30   ` [PATCH 6.1.y v3 2/2] mm/damon/core: fix unconditionally skip last region SJ Park
@ 2026-10-10  6:46     ` sashiko-bot
  2026-10-10  7:12       ` SJ Park
  0 siblings, 1 reply; 18+ messages in thread
From: sashiko-bot @ 2026-10-10  6:46 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: damon: use-after-free and permanent scheme disablement on target destroy
--

commit c97d3ff78284ee81310d99deee85b79f32b561d5
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon/core: fix unconditionally skip last region

Once quota set, the charge_{target,addr}_from unconditionally skips and
resets at the last region of the tracked target, so the last region can be
skipped even when it has not been processed.

Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index b5c85e6488596..1b9fb00405cfb 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -768,14 +769,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
>  		if (quota->charge_target_from) {
>  			if (t != quota->charge_target_from)
>  				continue;

[Severity: High]
Does this code have a use-after-free and permanent scheme disablement issue
when a target is destroyed?

If the user dynamically removes a target via sysfs, damon_sysfs_set_targets()
calls damon_destroy_target() which frees the target memory:

mm/damon/sysfs.c:damon_sysfs_set_targets() {
        ...
		} else {
			if (damon_target_has_pid(ctx))
				put_pid(t->pid);
			damon_destroy_target(t);
		}
        ...
}

mm/damon/core.c:damon_destroy_target() {
	damon_del_target(t);
	damon_free_target(t);
}

Existing schemes are updated in-place via damon_sysfs_update_scheme(), which
does not clear the charge_target_from pointer, leaving a dangling pointer in
the scheme.

Immediately after, when kdamond_apply_schemes() runs in the same thread,
damon_do_apply_schemes() compares the valid remaining targets against this
dangling pointer (if (t != quota->charge_target_from)). Since it does not
match the remaining targets, it unconditionally executes continue, skipping
the scheme for all regions.

Because a matching target is never reached, the pointer is never reset (the
code cannot reach the clean_charge_from_skip label). This results in
permanent disablement of the scheme and persistent use-after-free comparisons
against the dangling pointer.

> -			if (r == damon_last_region(t)) {
> -				quota->charge_target_from = NULL;
> -				quota->charge_addr_from = 0;
> -				continue;
> -			}
>  			if (quota->charge_addr_from &&
> -					r->ar.end <= quota->charge_addr_from)
> -				continue;
> +					r->ar.end <= quota->charge_addr_from) {
> +				skip = true;
> +				goto clean_charge_from_skip;
> +			}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010063042.8611-1-sj@kernel.org?part=2

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
  2026-10-10  6:41     ` sashiko-bot
@ 2026-10-10  7:11       ` SJ Park
  0 siblings, 0 replies; 18+ messages in thread
From: SJ Park @ 2026-10-10  7:11 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 10 Oct 2026 06:41:56 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm/damon: double-application of earlier schemes to newly split regions
> --
> 
> commit 01d768c4d318527360ef5913771d20f22a99984a
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
> 
> kdamond_apply_schemes() is using damon_for_each_region_safe(), which is
> safe for deallocation of the region inside the loop. However, the loop
> internal logic does not deallocate regions. Hence it is only wasting the
> next pointer.
> 
> Link: https://lkml.kernel.org/r/20260227170623.95384-3-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 7afbc20833ea4..b5c85e6488596 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -912,7 +911,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
> >  	}
> >  
> >  	damon_for_each_target(t, c) {
> > -		damon_for_each_region_safe(r, next_r, t)
> > +		damon_for_each_region(r, t)
> >  			damon_do_apply_schemes(c, t, r);
> 
> [Severity: High]
> Does this code cause earlier schemes to be applied twice to newly split
> regions?
> 
> If multiple schemes are configured, an earlier scheme (e.g. Scheme 0) might
> apply its action to a full, un-split region. If a later scheme (e.g.
> Scheme 1) then decides to split the region, damon_split_region_at() inserts
> the newly split part into the list immediately after the current region.
> 
> Because this loop in kdamond_apply_schemes() now uses
> damon_for_each_region() instead of the safe variant, the outer loop will
> advance to this newly split region and call damon_do_apply_schemes() on it
> again.
> 
> Will this cause damon_do_apply_schemes() to evaluate the new region from
> the beginning of the scheme list, resulting in Scheme 0 executing its
> memory management actions (like madvise) and counting statistics a second
> time on the same memory range?

The same issue exists in the mainline.  The user impact is minor.  No blocker
of this patch in my opinion.  I will separately work on it.

> 
> >  	}
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261010063042.8611-1-sj@kernel.org?part=1


Thanks,
SJ

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6.1.y v3 2/2] mm/damon/core: fix unconditionally skip last region
  2026-10-10  6:46     ` sashiko-bot
@ 2026-10-10  7:12       ` SJ Park
  0 siblings, 0 replies; 18+ messages in thread
From: SJ Park @ 2026-10-10  7:12 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 10 Oct 2026 06:46:00 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm: damon: use-after-free and permanent scheme disablement on target destroy
> --
> 
> commit c97d3ff78284ee81310d99deee85b79f32b561d5
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: fix unconditionally skip last region
> 
> Once quota set, the charge_{target,addr}_from unconditionally skips and
> resets at the last region of the tracked target, so the last region can be
> skipped even when it has not been processed.
> 
> Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index b5c85e6488596..1b9fb00405cfb 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -768,14 +769,11 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
> >  		if (quota->charge_target_from) {
> >  			if (t != quota->charge_target_from)
> >  				continue;
> 
> [Severity: High]
> Does this code have a use-after-free and permanent scheme disablement issue
> when a target is destroyed?
> 
> If the user dynamically removes a target via sysfs, damon_sysfs_set_targets()
> calls damon_destroy_target() which frees the target memory:
> 
> mm/damon/sysfs.c:damon_sysfs_set_targets() {
>         ...
> 		} else {
> 			if (damon_target_has_pid(ctx))
> 				put_pid(t->pid);
> 			damon_destroy_target(t);
> 		}
>         ...
> }
> 
> mm/damon/core.c:damon_destroy_target() {
> 	damon_del_target(t);
> 	damon_free_target(t);
> }
> 
> Existing schemes are updated in-place via damon_sysfs_update_scheme(), which
> does not clear the charge_target_from pointer, leaving a dangling pointer in
> the scheme.
> 
> Immediately after, when kdamond_apply_schemes() runs in the same thread,
> damon_do_apply_schemes() compares the valid remaining targets against this
> dangling pointer (if (t != quota->charge_target_from)). Since it does not
> match the remaining targets, it unconditionally executes continue, skipping
> the scheme for all regions.
> 
> Because a matching target is never reached, the pointer is never reset (the
> code cannot reach the clean_charge_from_skip label). This results in
> permanent disablement of the scheme and persistent use-after-free comparisons
> against the dangling pointer.

This is a pre-existing issue that also exists in the mainline.  I will
separately work on this.

> 
> > -			if (r == damon_last_region(t)) {
> > -				quota->charge_target_from = NULL;
> > -				quota->charge_addr_from = 0;
> > -				continue;
> > -			}
> >  			if (quota->charge_addr_from &&
> > -					r->ar.end <= quota->charge_addr_from)
> > -				continue;
> > +					r->ar.end <= quota->charge_addr_from) {
> > +				skip = true;
> > +				goto clean_charge_from_skip;
> > +			}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261010063042.8611-1-sj@kernel.org?part=2


Thanks,
SJ

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2026-10-10  7:12 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <2026092949-surpass-thimble-02a8@gregkh>
2026-09-30  9:48 ` [PATCH 6.1.y] mm/damon/core: fix unconditionally skip last region SJ Park
2026-09-30 10:05   ` sashiko-bot
2026-09-30 10:23     ` SJ Park
2026-10-02 14:19   ` Sasha Levin
2026-10-09 14:48 ` [PATCH 6.1.y v2 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
2026-10-09 14:48   ` [PATCH 6.1.y v2 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
2026-10-09 14:56     ` sashiko-bot
2026-10-09 15:09       ` SJ Park
2026-10-09 14:48   ` [PATCH 6.1.y v2 2/2] mm/damon/core: fix unconditionally skip last region SJ Park
2026-10-09 15:04     ` sashiko-bot
2026-10-09 15:11       ` SJ Park
2026-10-10  6:30 ` [PATCH 6.1.y v3 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions() SJ Park
2026-10-10  6:30   ` [PATCH 6.1.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes() SJ Park
2026-10-10  6:41     ` sashiko-bot
2026-10-10  7:11       ` SJ Park
2026-10-10  6:30   ` [PATCH 6.1.y v3 2/2] mm/damon/core: fix unconditionally skip last region SJ Park
2026-10-10  6:46     ` sashiko-bot
2026-10-10  7:12       ` SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox