[PATCH 6.6.y] mm/damon/core: fix unconditionally skip last region

[PATCH 6.6.y] mm/damon/core: fix unconditionally skip last region

[SJ Park]

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 | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 0713fda22682c..5ede1733369f5 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -883,19 +883,17 @@ static bool damos_skip_charged_region(struct damon_target *t,
 	struct damon_region *r = *rp;
 	struct damos_quota *quota = &s->quota;
 	unsigned long sz_to_skip;
+	bool skip = false;
 
 	/* Skip previously charged regions */
 	if (quota->charge_target_from) {
 		if (t != quota->charge_target_from)
 			return true;
-		if (r == damon_last_region(t)) {
-			quota->charge_target_from = NULL;
-			quota->charge_addr_from = 0;
-			return true;
-		}
 		if (quota->charge_addr_from &&
-				r->ar.end <= quota->charge_addr_from)
-			return true;
+				r->ar.end <= quota->charge_addr_from) {
+			skip = true;
+			goto out;
+		}
 
 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
@@ -909,11 +907,15 @@ static bool damos_skip_charged_region(struct damon_target *t,
 			damon_split_region_at(t, r, sz_to_skip);
 			r = damon_next_region(r);
 			*rp = r;
+			skip = false;
 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}
-	return false;
+	return skip;
 }
 
 static void damos_update_stat(struct damos *s,
-- 
2.47.3

Re: [PATCH 6.6.y] mm/damon/core: fix unconditionally skip last region

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: invalid list head dereference if damon_split_region_at() fails
- [Medium] mm: scheme state leak due to early return for small last region
--

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

mm/damon/core: fix unconditionally skip last region

This commit modifies damos_skip_charged_region() to only reset the charge
state when the last region is reached. It ensures the last region is skipped
only when it is applied or cannot be split.

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

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 0713fda22682c..5ede1733369f5 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

>  		if (quota->charge_addr_from && r->ar.start <
>  				quota->charge_addr_from) {
>  			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
>  					r->ar.start, DAMON_MIN_REGION);
>  			if (!sz_to_skip) {
>  				if (damon_sz_region(r) <= DAMON_MIN_REGION)
>  					return true;

[Severity: Medium]
Does this early return for a small last region bypass the state clearing at
the relocated out label?

If damos_skip_charged_region() processes the last region in a target and it
is smaller than DAMON_MIN_REGION, the code executes return true early. This
bypasses the out block that clears quota->charge_target_from.

In the subsequent window, the scheme could again skip all targets until the
saved target, evaluate its last region, and hit the same early return,
potentially skipping targets endlessly across windows.

>  				sz_to_skip = DAMON_MIN_REGION;
>  			}
>  			damon_split_region_at(t, r, sz_to_skip);
>  			r = damon_next_region(r);
>  			*rp = r;

[Severity: High]
Can unconditionally advancing the region pointer after a potentially failed
split on the last element cause list head type confusion?

When damos_skip_charged_region() processes the last region of a target and
attempts to split it, it calls damon_split_region_at(). If the split fails
due to a memory allocation failure, the region is not split and remains the
last element in the list.

The subsequent call:
r = damon_next_region(r);

will dereference r->list.next, which points to the list head
(&t->regions_list), returning a type-confused pointer.

Passing this bogus pointer back via *rp = r means that subsequent calls to
damos_valid_target() and damos_apply_scheme() will dereference it, which
could lead to a heap out-of-bounds access.

> +			skip = false;
>  		}
> +	}
> +out:
> +	if (r == damon_last_region(t)) {
>  		quota->charge_target_from = NULL;
>  		quota->charge_addr_from = 0;
>  	}

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

Re: [PATCH 6.6.y] mm/damon/core: fix unconditionally skip last region

[SJ Park]

On Wed, 30 Sep 2026 09:54:41 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] mm: invalid list head dereference if damon_split_region_at() fails
> - [Medium] mm: scheme state leak due to early return for small last region
> --
> 
> commit ae1e4dc8429f6a152575d9c64a1876f9dfb3b7ad
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: fix unconditionally skip last region
> 
> This commit modifies damos_skip_charged_region() to only reset the charge
> state when the last region is reached. It ensures the last region is skipped
> only when it is applied or cannot be split.
> 
> Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 0713fda22682c..5ede1733369f5 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> >  		if (quota->charge_addr_from && r->ar.start <
> >  				quota->charge_addr_from) {
> >  			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
> >  					r->ar.start, DAMON_MIN_REGION);
> >  			if (!sz_to_skip) {
> >  				if (damon_sz_region(r) <= DAMON_MIN_REGION)
> >  					return true;
> 
> [Severity: Medium]
> Does this early return for a small last region bypass the state clearing at
> the relocated out label?
> 
> If damos_skip_charged_region() processes the last region in a target and it
> is smaller than DAMON_MIN_REGION, the code executes return true early. This
> bypasses the out block that clears quota->charge_target_from.
> 
> In the subsequent window, the scheme could again skip all targets until the
> saved target, evaluate its last region, and hit the same early return,
> potentially skipping targets endlessly across windows.

Good catch, I forgot converting this while backporting.  I will send v2.

> 
> >  				sz_to_skip = DAMON_MIN_REGION;
> >  			}
> >  			damon_split_region_at(t, r, sz_to_skip);
> >  			r = damon_next_region(r);
> >  			*rp = r;
> 
> [Severity: High]
> Can unconditionally advancing the region pointer after a potentially failed
> split on the last element cause list head type confusion?
> 
> When damos_skip_charged_region() processes the last region of a target and
> attempts to split it, it calls damon_split_region_at(). If the split fails
> due to a memory allocation failure, the region is not split and remains the
> last element in the list.
> 
> The subsequent call:
> r = damon_next_region(r);
> 
> will dereference r->list.next, which points to the list head
> (&t->regions_list), returning a type-confused pointer.
> 
> Passing this bogus pointer back via *rp = r means that subsequent calls to
> damos_valid_target() and damos_apply_scheme() will dereference it, which
> could lead to a heap out-of-bounds access.

Good catch, but pre-existing issue.  Maybe a fix that made upstream should be
applied here.  I will separately work on this.

> 
> > +			skip = false;
> >  		}
> > +	}
> > +out:
> > +	if (r == damon_last_region(t)) {
> >  		quota->charge_target_from = NULL;
> >  		quota->charge_addr_from = 0;
> >  	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260930093707.48739-1-sj@kernel.org?part=1


Thanks,
SJ

Re: [PATCH 6.6.y] mm/damon/core: fix unconditionally skip last region

[SJ Park]

Hello stable team,


Please ignore this patch.  I made a mistake while backporting.  I will send v2.
Sashiko found the mistake.  Please refer to my reply [1] to Sashiko for more
details.

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


Thanks,
SJ

[...]

[PATCH 6.6.y v2] mm/damon/core: fix unconditionally skip last region

[SJ Park]

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>
---
Changes from v1
- v1: https://lore.kernel.org/20260930093707.48739-1-sj@kernel.org
- Add missed damon_sz_region(r) <= min_region_sz case change.

 mm/damon/core.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 0713fda22682c..c31683c12dd48 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -883,37 +883,41 @@ static bool damos_skip_charged_region(struct damon_target *t,
 	struct damon_region *r = *rp;
 	struct damos_quota *quota = &s->quota;
 	unsigned long sz_to_skip;
+	bool skip = false;
 
 	/* Skip previously charged regions */
 	if (quota->charge_target_from) {
 		if (t != quota->charge_target_from)
 			return true;
-		if (r == damon_last_region(t)) {
-			quota->charge_target_from = NULL;
-			quota->charge_addr_from = 0;
-			return true;
-		}
 		if (quota->charge_addr_from &&
-				r->ar.end <= quota->charge_addr_from)
-			return true;
+				r->ar.end <= quota->charge_addr_from) {
+			skip = true;
+			goto out;
+		}
 
 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
 			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
 					r->ar.start, DAMON_MIN_REGION);
 			if (!sz_to_skip) {
-				if (damon_sz_region(r) <= DAMON_MIN_REGION)
-					return true;
+				if (damon_sz_region(r) <= DAMON_MIN_REGION) {
+					skip = true;
+					goto out;
+				}
 				sz_to_skip = DAMON_MIN_REGION;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
 			r = damon_next_region(r);
 			*rp = r;
+			skip = false;
 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}
-	return false;
+	return skip;
 }
 
 static void damos_update_stat(struct damos *s,
-- 
2.47.3

Re: [PATCH 6.6.y v2] mm/damon/core: fix unconditionally skip last region

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/damon: list head type confusion and memory corruption on allocation failure
--

commit 47762d8f5b1f74489d2b1ca149de65b77ac867a2
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. 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

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 0713fda22682c..c31683c12dd48 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -883,37 +883,41 @@ static bool damos_skip_charged_region(struct damon_target *t,
>  	struct damon_region *r = *rp;
>  	struct damos_quota *quota = &s->quota;
>  	unsigned long sz_to_skip;
> +	bool skip = false;
>  
>  	/* Skip previously charged regions */
>  	if (quota->charge_target_from) {
>  		if (t != quota->charge_target_from)
>  			return true;

[ ... ]

>  		if (quota->charge_addr_from && r->ar.start <
>  				quota->charge_addr_from) {
>  			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
>  					r->ar.start, DAMON_MIN_REGION);
>  			if (!sz_to_skip) {
> -				if (damon_sz_region(r) <= DAMON_MIN_REGION)
> -					return true;
> +				if (damon_sz_region(r) <= DAMON_MIN_REGION) {
> +					skip = true;
> +					goto out;
> +				}
>  				sz_to_skip = DAMON_MIN_REGION;
>  			}
>  			damon_split_region_at(t, r, sz_to_skip);
>  			r = damon_next_region(r);
>  			*rp = r;
> +			skip = false;

[Severity: High]
If damon_split_region_at() fails to allocate memory for the new region, it
silently returns without splitting. When r is the last region,
damon_next_region() wraps around to the list head, resulting in an invalid
struct damon_region pointer. Can this cause an invalid pointer to be
assigned to *rp?

>  		}
> +	}
> +out:
> +	if (r == damon_last_region(t)) {
>  		quota->charge_target_from = NULL;
>  		quota->charge_addr_from = 0;
>  	}
> -	return false;
> +	return skip;
>  }

[Severity: High]
If the allocation failed and r evaluates to a bogus list head pointer, this
check for damon_last_region() will fail. The function will then return
skip = false, causing the caller in damon_do_apply_schemes() to proceed
with the invalid pointer.

Could this lead to out-of-bounds reads in damos_valid_target() and
out-of-bounds writes in damos_apply_scheme() when the pointer is
dereferenced?

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

Re: [PATCH 6.6.y v2] mm/damon/core: fix unconditionally skip last region

[SJ Park]

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

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm/damon: list head type confusion and memory corruption on allocation failure
> --
> 
> commit 47762d8f5b1f74489d2b1ca149de65b77ac867a2
> 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. 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
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 0713fda22682c..c31683c12dd48 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -883,37 +883,41 @@ static bool damos_skip_charged_region(struct damon_target *t,
> >  	struct damon_region *r = *rp;
> >  	struct damos_quota *quota = &s->quota;
> >  	unsigned long sz_to_skip;
> > +	bool skip = false;
> >  
> >  	/* Skip previously charged regions */
> >  	if (quota->charge_target_from) {
> >  		if (t != quota->charge_target_from)
> >  			return true;
> 
> [ ... ]
> 
> >  		if (quota->charge_addr_from && r->ar.start <
> >  				quota->charge_addr_from) {
> >  			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
> >  					r->ar.start, DAMON_MIN_REGION);
> >  			if (!sz_to_skip) {
> > -				if (damon_sz_region(r) <= DAMON_MIN_REGION)
> > -					return true;
> > +				if (damon_sz_region(r) <= DAMON_MIN_REGION) {
> > +					skip = true;
> > +					goto out;
> > +				}
> >  				sz_to_skip = DAMON_MIN_REGION;
> >  			}
> >  			damon_split_region_at(t, r, sz_to_skip);
> >  			r = damon_next_region(r);
> >  			*rp = r;
> > +			skip = false;
> 
> [Severity: High]
> If damon_split_region_at() fails to allocate memory for the new region, it
> silently returns without splitting. When r is the last region,
> damon_next_region() wraps around to the list head, resulting in an invalid
> struct damon_region pointer. Can this cause an invalid pointer to be
> assigned to *rp?
> 
> >  		}
> > +	}
> > +out:
> > +	if (r == damon_last_region(t)) {
> >  		quota->charge_target_from = NULL;
> >  		quota->charge_addr_from = 0;
> >  	}
> > -	return false;
> > +	return skip;
> >  }
> 
> [Severity: High]
> If the allocation failed and r evaluates to a bogus list head pointer, this
> check for damon_last_region() will fail. The function will then return
> skip = false, causing the caller in damon_do_apply_schemes() to proceed
> with the invalid pointer.
> 
> Could this lead to out-of-bounds reads in damos_valid_target() and
> out-of-bounds writes in damos_apply_scheme() when the pointer is
> dereferenced?

I will separately work on this.  Refer to my other reply to same issue report
for details.

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

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


Thanks,
SJ

Re: [PATCH 6.6.y v2] mm/damon/core: fix unconditionally skip last region

[Sasha Levin]

> 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

Re: [PATCH 6.6.y v2] mm/damon/core: fix unconditionally skip last region

[SJ Park]

On Fri,  2 Oct 2026 10:19:50 -0400 Sasha Levin <sashal@kernel.org> wrote:

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

Makes perfect sense to me.  I will post the fix with this patch.


Thanks,
SJ

[...]