mm/damon/ops-common: call folio_test_lru() after folio_get()

RFC PATCH In review SeongJae Park 2 participants 4 messages May 23

Subsystems (3)

Stable branch Memory management DAMON

lore

Share & tools

lore

SeongJae Park

author maintainer

2026-05-23 19:42 UTC #0 lore raw

RFC PATCH 1/1 mm/damon/ops-common: call folio_test_lru() after folio_get()

damon_get_folio() speculatively calls folio_test_lru() before folio_try_get(). The folio can get freed and reallocated to a tail page. In the case, VM_BUG_ON_PGFLAGS() in const_folio_flags() can be triggered. Remove the speculative call.

Also do the folio_test_lru() check right after folio_try_get() success, since it is more likely than folio realloc race.

The race should be rare. Also the problem can happen only if the kernel has enabled CONFIG_DEBUG_VM_PGFLAGS. No real world report of this issue has been made so far. This fix is based on only theoretical analysis. That said, a bug is a bug. A similar issue was also fixed via commit 3203b3ab0fcf ("mm/filemap: don't call folio_test_locked() without a reference in next_uptodate_folio()"). I don't expect this change will make a meaningful impact to DAMON performance in the real world, though I will be happy to be corrected from the real world reports.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/[email protected]

Fixes 3f49584b262c ("mm/damon: implement primitives for the virtual memory address spaces")

Cc <[email protected]> # 5.15.x

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

---
 mm/damon/ops-common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

mm/damon/ops-common.c +2 -2

index 3a0ddc3ac7196..d3404615f9b75 100644
@@ -32,9 +32,9 @@ struct folio *damon_get_folio(unsigned long pfn)
 		return NULL;
 
 	folio = page_folio(page);
-	if (!folio_test_lru(folio) || !folio_try_get(folio))
+	if (!folio_try_get(folio))
 		return NULL;
-	if (unlikely(page_folio(page) != folio || !folio_test_lru(folio))) {
+	if (!folio_test_lru(folio) || unlikely(page_folio(page) != folio)) {
 		folio_put(folio);
 		folio = NULL;
 	}

base-commit: a94d68c2dfd523cebb2755787fb01c08eef70c43

Signature · 1 line

2.47.3

[email protected]

AI review

2026-05-23 19:57 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Speculative folio flag access occurs before pointer re-validation, triggering VM_BUG_ON_PGFLAGS()…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Speculative folio flag access occurs before pointer re-validation, triggering VM_BUG_ON_PGFLAGS() on tail pages. --

commit 119755fb3390d2b7d6c7f2278054cc95e95531c6
Author: SeongJae Park <[email protected]>

mm/damon/ops-common: call folio_test_lru() after folio_get()

This patch attempts to fix a potential VM_BUG_ON_PGFLAGS crash by removing the speculative LRU flag check before trying to get a reference in damon_get_folio(). It moves the LRU check after a successful try_get.

Link https://lore.kernel.org/[email protected] open here

mm/damon/ops-common.c struct folio *damon_get_folio(unsigned long pfn) · line 37

Quoted 13 lines

mm/damon/ops-common.c +2 -2

index 3a0ddc3ac7196..d3404615f9b75 100644
@@ -32,9 +32,9 @@ struct folio *damon_get_folio(unsigned long pfn)
 		return NULL;
 
 	folio = page_folio(page);
-	if (!folio_test_lru(folio) || !folio_try_get(folio))
+	if (!folio_try_get(folio))
 		return NULL;
-	if (unlikely(page_folio(page) != folio || !folio_test_lru(folio))) {
+	if (!folio_test_lru(folio) || unlikely(page_folio(page) != folio)) {

[Severity: High]
Does this sequence reintroduce the same bug the patch intends to fix?

By placing the LRU flag check before the pointer re-validation, C short-circuit evaluation will evaluate the LRU flag first.

If the page was concurrently freed and reallocated as a tail page, folio_try_get() might still succeed. Evaluating the LRU flag on this unvalidated tail page will trigger the VM_BUG_ON_PGFLAGS() in const_folio_flags().

Should the pointer re-validation happen before accessing any folio flags in PFN scanners?

Quoted 3 lines

mm/damon/ops-common.c +0 -0

 		folio_put(folio);
 		folio = NULL;
 	}

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

SeongJae Park

author maintainer

2026-05-23 20:26 UTC #2 lore raw

Quoted 35 lines · On Sat, 23 May 2026 19:57:06 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Speculative folio flag access occurs before pointer re-validation, triggering VM_BUG_ON_PGFLAGS() on tail pages. --
commit 119755fb3390d2b7d6c7f2278054cc95e95531c6
Author: SeongJae Park <[email protected]>
mm/damon/ops-common: call folio_test_lru() after folio_get()
This patch attempts to fix a potential VM_BUG_ON_PGFLAGS crash by removing the speculative LRU flag check before trying to get a reference in damon_get_folio(). It moves the LRU check after a successful try_get.
Link https://lore.kernel.org/[email protected] open here
mm/damon/ops-common.c struct folio *damon_get_folio(unsigned long pfn) · line 37
Quoted 13 lines
mm/damon/ops-common.c +2 -2
index 3a0ddc3ac7196..d3404615f9b75 100644
@@ -32,9 +32,9 @@ struct folio *damon_get_folio(unsigned long pfn)
 		return NULL;
 
 	folio = page_folio(page);
-	if (!folio_test_lru(folio) || !folio_try_get(folio))
+	if (!folio_try_get(folio))
 		return NULL;
-	if (unlikely(page_folio(page) != folio || !folio_test_lru(folio))) {
+	if (!folio_test_lru(folio) || unlikely(page_folio(page) != folio)) {
[Severity: High]
Does this sequence reintroduce the same bug the patch intends to fix?
By placing the LRU flag check before the pointer re-validation, C short-circuit evaluation will evaluate the LRU flag first.

Quoted 8 lines

If the page was concurrently freed and reallocated as a tail page, folio_try_get() might still succeed.
Evaluating the LRU flag on this
unvalidated tail page will trigger the VM_BUG_ON_PGFLAGS() in const_folio_flags().
Should the pointer re-validation happen before accessing any folio flags in PFN scanners?

I was initially assume folio_try_get() will always return a valid folio. But I find no code to avoid Sashiko's theory in folio_try_get().

So, I think Sashiko is correct. I will drop the above line change on the next revision.

Thanks,
SJ

[...]

SeongJae Park

author maintainer

2026-05-23 20:31 UTC #3 lore raw

Quoted 43 lines · On Sat, 23 May 2026 13:26:07 -0700 SeongJae Park <[email protected]> wrote:

Quoted 41 lines · On Sat, 23 May 2026 19:57:06 +0000 [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Speculative folio flag access occurs before pointer re-validation, triggering VM_BUG_ON_PGFLAGS() on tail pages. --
commit 119755fb3390d2b7d6c7f2278054cc95e95531c6
Author: SeongJae Park <[email protected]>
mm/damon/ops-common: call folio_test_lru() after folio_get()
This patch attempts to fix a potential VM_BUG_ON_PGFLAGS crash by removing the speculative LRU flag check before trying to get a reference in damon_get_folio(). It moves the LRU check after a successful try_get.
Link https://lore.kernel.org/[email protected] open here
mm/damon/ops-common.c struct folio *damon_get_folio(unsigned long pfn) · line 37
Quoted 13 lines
mm/damon/ops-common.c +2 -2
index 3a0ddc3ac7196..d3404615f9b75 100644
@@ -32,9 +32,9 @@ struct folio *damon_get_folio(unsigned long pfn)
 		return NULL;
 
 	folio = page_folio(page);
-	if (!folio_test_lru(folio) || !folio_try_get(folio))
+	if (!folio_try_get(folio))
 		return NULL;
-	if (unlikely(page_folio(page) != folio || !folio_test_lru(folio))) {
+	if (!folio_test_lru(folio) || unlikely(page_folio(page) != folio)) {
[Severity: High]
Does this sequence reintroduce the same bug the patch intends to fix?
By placing the LRU flag check before the pointer re-validation, C short-circuit evaluation will evaluate the LRU flag first.
If the page was concurrently freed and reallocated as a tail page, folio_try_get() might still succeed.
Evaluating the LRU flag on this
unvalidated tail page will trigger the VM_BUG_ON_PGFLAGS() in const_folio_flags().

Quoted 8 lines

Quoted 2 lines
Should the pointer re-validation happen before accessing any folio flags in PFN scanners?
I was initially assume folio_try_get() will always return a valid folio. But I find no code to avoid Sashiko's theory in folio_try_get().
So, I think Sashiko is correct. I will drop the above line change on the next revision.

But, I may still mark the folio_test_lru() as no more unlikely.

Thanks,
SJ

[...]