* [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 1 sibling, 2 replies; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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 1 sibling, 2 replies; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ messages in thread
end of thread, other threads:[~2026-10-09 15:11 UTC | newest]
Thread overview: 11+ 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox