* [PATCH v2 0/2] mm/pagewalk: fix stale walk->action escaping walk_pmd_range() @ 2026-08-11 16:18 Hyunwoo Kim 2026-08-11 16:18 ` [PATCH v2 1/2] " Hyunwoo Kim 2026-08-11 16:18 ` [PATCH v2 2/2] selftests/mm: add stale walk->action race test Hyunwoo Kim 0 siblings, 2 replies; 11+ messages in thread From: Hyunwoo Kim @ 2026-08-11 16:18 UTC (permalink / raw) To: akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko Cc: linux-mm, linux-kernel, imv4bel walk_pmd_range() could return with walk->action left at ACTION_AGAIN, which causes walk_pud_range() to retry the PUD walk and invoke the walk callbacks twice over the same range. Patch 1 is the fix. Patch 2 adds a regression test for it. Changes in v2: - 1/2: reworked the changelog along the lines Lorenzo suggested and said how the bug was found; the fix itself is unchanged (Lorenzo) - 2/2: new, races smaps against MADV_DONTNEED and checks that Rss does not come out twice as large as what was faulted in (Lorenzo) - v1: https://lore.kernel.org/r/anmdrYGVqM-U4vlo@v4bel Hyunwoo Kim (2): mm/pagewalk: fix stale walk->action escaping walk_pmd_range() selftests/mm: add stale walk->action race test mm/pagewalk.c | 6 +- tools/testing/selftests/mm/.gitignore | 1 + tools/testing/selftests/mm/Makefile | 2 + tools/testing/selftests/mm/ksft_pagewalk.sh | 4 + .../testing/selftests/mm/pagewalk_race_test.c | 138 ++++++++++++++++++ tools/testing/selftests/mm/run_vmtests.sh | 2 + tools/testing/selftests/mm/vm_util.h | 1 + 7 files changed, 150 insertions(+), 4 deletions(-) create mode 100755 tools/testing/selftests/mm/ksft_pagewalk.sh create mode 100644 tools/testing/selftests/mm/pagewalk_race_test.c -- 2.43.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] mm/pagewalk: fix stale walk->action escaping walk_pmd_range() 2026-08-11 16:18 [PATCH v2 0/2] mm/pagewalk: fix stale walk->action escaping walk_pmd_range() Hyunwoo Kim @ 2026-08-11 16:18 ` Hyunwoo Kim 2026-08-11 16:23 ` Lorenzo Stoakes (ARM) ` (2 more replies) 2026-08-11 16:18 ` [PATCH v2 2/2] selftests/mm: add stale walk->action race test Hyunwoo Kim 1 sibling, 3 replies; 11+ messages in thread From: Hyunwoo Kim @ 2026-08-11 16:18 UTC (permalink / raw) To: akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko Cc: linux-mm, linux-kernel, imv4bel, stable If ->pmd_entry() sets walk->action = ACTION_AGAIN, the pmd_none() check is retried. The PMD entry may be cleared at the point of retry. In this case, if walk->ops->install_pte is not specified, the code continues to the next PMD entry in the range without resetting walk->action to ACTION_SUBTREE. This leaves walk->action erroneously set to ACTION_AGAIN, which is incorrect. This was incorrect but not problematic up until commit 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault") which updated walk_pud_range() to check for walk->action == ACTION_AGAIN upon walk_pmd_range()'s return, causing the PUD walk to be retried. In this case this results in duplicate walk callbacks being invoked, which is erroneous and will break any caller that is not idempotent with respect to this (and waste time for those which are). A specific example of this breaking things is mincore which walks an internal cursor data structure a byte at a time on assumption that page table entry callbacks are called only once for each entry. Fix the problem by resetting walk->action to ACTION_SUBTREE prior to the none check. The pattern also exists in walk_pud_range() so fix it there too. This issue was found through AI-based fuzzing. Fixes: 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5 Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com> --- mm/pagewalk.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/mm/pagewalk.c b/mm/pagewalk.c index 5d87c632a25507..d3bfece3193366 100644 --- a/mm/pagewalk.c +++ b/mm/pagewalk.c @@ -126,6 +126,7 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end, pmd = pmd_offset(pud, addr); do { again: + walk->action = ACTION_SUBTREE; next = pmd_addr_end(addr, end); if (pmd_none(*pmd)) { if (has_install) @@ -138,8 +139,6 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end, continue; } - walk->action = ACTION_SUBTREE; - /* * This implies that each ->pmd_entry() handler * needs to know about pmd_trans_huge() pmds @@ -196,6 +195,7 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end, pud = pud_offset(p4d, addr); do { again: + walk->action = ACTION_SUBTREE; next = pud_addr_end(addr, end); if (pud_none(*pud)) { if (has_install) @@ -208,8 +208,6 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end, continue; } - walk->action = ACTION_SUBTREE; - if (ops->pud_entry) err = ops->pud_entry(pud, addr, next, walk); if (err) -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] mm/pagewalk: fix stale walk->action escaping walk_pmd_range() 2026-08-11 16:18 ` [PATCH v2 1/2] " Hyunwoo Kim @ 2026-08-11 16:23 ` Lorenzo Stoakes (ARM) 2026-08-11 17:10 ` David Hildenbrand (Arm) 2026-08-11 19:37 ` Andrew Morton 2 siblings, 0 replies; 11+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-08-11 16:23 UTC (permalink / raw) To: Hyunwoo Kim Cc: akpm, david, liam, vbabka, rppt, surenb, mhocko, linux-mm, linux-kernel, stable On Wed, Aug 12, 2026 at 01:18:57AM +0900, Hyunwoo Kim wrote: > If ->pmd_entry() sets walk->action = ACTION_AGAIN, the pmd_none() > check is retried. The PMD entry may be cleared at the point of retry. > > In this case, if walk->ops->install_pte is not specified, the code > continues to the next PMD entry in the range without resetting > walk->action to ACTION_SUBTREE. > > This leaves walk->action erroneously set to ACTION_AGAIN, which is > incorrect. > > This was incorrect but not problematic up until commit 3b89863c3fa4 > ("mm/pagewalk: fix race between concurrent split and refault") > which updated walk_pud_range() to check for walk->action == > ACTION_AGAIN upon walk_pmd_range()'s return, causing the PUD walk > to be retried. > > In this case this results in duplicate walk callbacks being > invoked, which is erroneous and will break any caller that is not > idempotent with respect to this (and waste time for those which > are). > > A specific example of this breaking things is mincore which walks > an internal cursor data structure a byte at a time on assumption > that page table entry callbacks are called only once for each > entry. > > Fix the problem by resetting walk->action to ACTION_SUBTREE prior > to the none check. > > The pattern also exists in walk_pud_range() so fix it there too. > > This issue was found through AI-based fuzzing. > > Fixes: 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault") > Cc: stable@vger.kernel.org > Assisted-by: Claude:claude-opus-5 > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com> Thanks, LGTM so: Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org> > --- > mm/pagewalk.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/mm/pagewalk.c b/mm/pagewalk.c > index 5d87c632a25507..d3bfece3193366 100644 > --- a/mm/pagewalk.c > +++ b/mm/pagewalk.c > @@ -126,6 +126,7 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end, > pmd = pmd_offset(pud, addr); > do { > again: > + walk->action = ACTION_SUBTREE; > next = pmd_addr_end(addr, end); > if (pmd_none(*pmd)) { > if (has_install) > @@ -138,8 +139,6 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end, > continue; > } > > - walk->action = ACTION_SUBTREE; > - > /* > * This implies that each ->pmd_entry() handler > * needs to know about pmd_trans_huge() pmds > @@ -196,6 +195,7 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end, > pud = pud_offset(p4d, addr); > do { > again: > + walk->action = ACTION_SUBTREE; > next = pud_addr_end(addr, end); > if (pud_none(*pud)) { > if (has_install) > @@ -208,8 +208,6 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end, > continue; > } > > - walk->action = ACTION_SUBTREE; > - > if (ops->pud_entry) > err = ops->pud_entry(pud, addr, next, walk); > if (err) > -- > 2.43.0 > -- Cheers, Lorenzo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] mm/pagewalk: fix stale walk->action escaping walk_pmd_range() 2026-08-11 16:18 ` [PATCH v2 1/2] " Hyunwoo Kim 2026-08-11 16:23 ` Lorenzo Stoakes (ARM) @ 2026-08-11 17:10 ` David Hildenbrand (Arm) 2026-08-11 19:37 ` Andrew Morton 2 siblings, 0 replies; 11+ messages in thread From: David Hildenbrand (Arm) @ 2026-08-11 17:10 UTC (permalink / raw) To: Hyunwoo Kim, akpm, ljs, liam, vbabka, rppt, surenb, mhocko Cc: linux-mm, linux-kernel, stable On 8/11/26 18:18, Hyunwoo Kim wrote: > If ->pmd_entry() sets walk->action = ACTION_AGAIN, the pmd_none() > check is retried. The PMD entry may be cleared at the point of retry. > > In this case, if walk->ops->install_pte is not specified, the code > continues to the next PMD entry in the range without resetting > walk->action to ACTION_SUBTREE. > > This leaves walk->action erroneously set to ACTION_AGAIN, which is > incorrect. > > This was incorrect but not problematic up until commit 3b89863c3fa4 > ("mm/pagewalk: fix race between concurrent split and refault") > which updated walk_pud_range() to check for walk->action == > ACTION_AGAIN upon walk_pmd_range()'s return, causing the PUD walk > to be retried. > > In this case this results in duplicate walk callbacks being > invoked, which is erroneous and will break any caller that is not > idempotent with respect to this (and waste time for those which > are). > > A specific example of this breaking things is mincore which walks > an internal cursor data structure a byte at a time on assumption > that page table entry callbacks are called only once for each > entry. > > Fix the problem by resetting walk->action to ACTION_SUBTREE prior > to the none check. > > The pattern also exists in walk_pud_range() so fix it there too. > > This issue was found through AI-based fuzzing. > > Fixes: 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault") > Cc: stable@vger.kernel.org > Assisted-by: Claude:claude-opus-5 > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com> > --- Acked-by: David Hildenbrand (Arm) <david@kernel.org> -- Cheers, David ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] mm/pagewalk: fix stale walk->action escaping walk_pmd_range() 2026-08-11 16:18 ` [PATCH v2 1/2] " Hyunwoo Kim 2026-08-11 16:23 ` Lorenzo Stoakes (ARM) 2026-08-11 17:10 ` David Hildenbrand (Arm) @ 2026-08-11 19:37 ` Andrew Morton 2026-08-11 20:17 ` Hyunwoo Kim 2 siblings, 1 reply; 11+ messages in thread From: Andrew Morton @ 2026-08-11 19:37 UTC (permalink / raw) To: Hyunwoo Kim Cc: david, ljs, liam, vbabka, rppt, surenb, mhocko, linux-mm, linux-kernel, stable, Max Boone On Wed, 12 Aug 2026 01:18:57 +0900 Hyunwoo Kim <imv4bel@gmail.com> wrote: > If ->pmd_entry() sets walk->action = ACTION_AGAIN, the pmd_none() > check is retried. The PMD entry may be cleared at the point of retry. > > In this case, if walk->ops->install_pte is not specified, the code > continues to the next PMD entry in the range without resetting > walk->action to ACTION_SUBTREE. > > This leaves walk->action erroneously set to ACTION_AGAIN, which is > incorrect. > > This was incorrect but not problematic up until commit 3b89863c3fa4 > ("mm/pagewalk: fix race between concurrent split and refault") > which updated walk_pud_range() to check for walk->action == > ACTION_AGAIN upon walk_pmd_range()'s return, causing the PUD walk > to be retried. > > In this case this results in duplicate walk callbacks being > invoked, which is erroneous and will break any caller that is not > idempotent with respect to this (and waste time for those which > are). "break". Please describe the breakage completely. It's really the most important information in the whole effort. IOW, when fixing a bug please describe the userspace-visible runtime effects of that bug. eg, what were the results of the fuzzer? Is there a Link:? A stack trace? > A specific example of this breaking things is mincore which walks > an internal cursor data structure a byte at a time on assumption > that page table entry callbacks are called only once for each > entry. > > Fix the problem by resetting walk->action to ACTION_SUBTREE prior > to the none check. > > The pattern also exists in walk_pud_range() so fix it there too. > > This issue was found through AI-based fuzzing. > > Fixes: 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault") It's good to cc the relevant Author(s). > Cc: stable@vger.kernel.org We really should tell -stable maintainers (and all other users of earlier kernels) all about the above things. > mm/pagewalk.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) This depends on the above info, but I'd prefer to process the bugfix promptly and defer consideration of the selftest until the next -rc cycle. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] mm/pagewalk: fix stale walk->action escaping walk_pmd_range() 2026-08-11 19:37 ` Andrew Morton @ 2026-08-11 20:17 ` Hyunwoo Kim 0 siblings, 0 replies; 11+ messages in thread From: Hyunwoo Kim @ 2026-08-11 20:17 UTC (permalink / raw) To: Andrew Morton Cc: david, ljs, liam, vbabka, rppt, surenb, mhocko, linux-mm, linux-kernel, stable, Max Boone, imv4bel On Tue, Aug 11, 2026 at 12:37:02PM -0700, Andrew Morton wrote: > On Wed, 12 Aug 2026 01:18:57 +0900 Hyunwoo Kim <imv4bel@gmail.com> wrote: > > > If ->pmd_entry() sets walk->action = ACTION_AGAIN, the pmd_none() > > check is retried. The PMD entry may be cleared at the point of retry. > > > > In this case, if walk->ops->install_pte is not specified, the code > > continues to the next PMD entry in the range without resetting > > walk->action to ACTION_SUBTREE. > > > > This leaves walk->action erroneously set to ACTION_AGAIN, which is > > incorrect. > > > > This was incorrect but not problematic up until commit 3b89863c3fa4 > > ("mm/pagewalk: fix race between concurrent split and refault") > > which updated walk_pud_range() to check for walk->action == > > ACTION_AGAIN upon walk_pmd_range()'s return, causing the PUD walk > > to be retried. > > > > In this case this results in duplicate walk callbacks being > > invoked, which is erroneous and will break any caller that is not > > idempotent with respect to this (and waste time for those which > > are). > > "break". Please describe the breakage completely. It's really the > most important information in the whole effort. > > IOW, when fixing a bug please describe the userspace-visible runtime > effects of that bug. > > eg, what were the results of the fuzzer? To be precise, this is an out-of-bounds write. > Is there a Link:? This came from a local fuzzer, so there is no Link: > A stack trace? [ 2.272695] ================================================================== [ 2.273471] BUG: KASAN: slab-out-of-bounds in __mincore_unmapped_range+0x14f/0x190 [ 2.274302] Write of size 1 at addr ffff888008d9b000 by task poc/106 [ 2.274966] [ 2.275154] CPU: 0 UID: 1000 PID: 106 Comm: poc Not tainted 7.2.0-rc6-00429-ga7c7074b58d2 #55 PREEMPT(lazy) [ 2.275159] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 2.275164] Call Trace: [ 2.275170] <TASK> [ 2.275172] dump_stack_lvl+0x53/0x70 [ 2.275200] print_report+0xd0/0x630 [ 2.275210] ? __pfx__raw_spin_lock_irqsave+0x10/0x10 [ 2.275219] ? irqentry_exit+0xd2/0x670 [ 2.275224] ? irqentry_exit+0xd2/0x670 [ 2.275226] ? __virt_addr_valid+0xef/0x1a0 [ 2.275239] ? __mincore_unmapped_range+0x14f/0x190 [ 2.275242] kasan_report+0xce/0x100 [ 2.275245] ? __mincore_unmapped_range+0x14f/0x190 [ 2.275248] __mincore_unmapped_range+0x14f/0x190 [ 2.275252] mincore_unmapped_range+0x45/0x70 [ 2.275254] walk_pgd_range+0xafc/0xfc0 [ 2.275261] ? __pfx_walk_pgd_range+0x10/0x10 [ 2.275264] ? __update_load_avg_se+0x3d1/0x670 [ 2.275275] __walk_page_range+0xc0/0x310 [ 2.275278] ? __pfx_find_vma+0x10/0x10 [ 2.275281] ? finish_task_switch.isra.0+0x16d/0x4f0 [ 2.275290] walk_page_range_mm_unsafe+0x26f/0x3a0 [ 2.275293] ? __pfx_mtree_load+0x10/0x10 [ 2.275298] ? __pfx_walk_page_range_mm_unsafe+0x10/0x10 [ 2.275302] ? __free_frozen_pages+0x54d/0x7e0 [ 2.275308] __do_sys_mincore+0x132/0x380 [ 2.275311] do_syscall_64+0xf9/0x540 [ 2.275316] entry_SYSCALL_64_after_hwframe+0x77/0x7f [ 2.275322] RIP: 0033:0x422ccd [ 2.275326] Code: b3 66 2e 0f 1f 84 00 00 00 00 00 66 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 [ 2.275329] RSP: 002b:00007fffffffec18 EFLAGS: 00000287 ORIG_RAX: 000000000000001b [ 2.275337] RAX: ffffffffffffffda RBX: 0000000000000066 RCX: 0000000000422ccd [ 2.275339] RDX: 00000000004d0940 RSI: 0000000001000000 RDI: 00007ffff4000000 [ 2.275340] RBP: 00000000004d0940 R08: 0000000000000100 R09: 0000000000000100 [ 2.275342] R10: 0000000000000100 R11: 0000000000000287 R12: 20c49ba5e353f7cf [ 2.275343] R13: 00000000004990d3 R14: 0000000000000000 R15: 0000000000000001 [ 2.275346] </TASK> [ 2.275347] [ 2.296904] The buggy address belongs to the object at ffff888008d9b000 [ 2.296904] which belongs to the cache sigqueue of size 80 [ 2.298151] The buggy address is located 0 bytes inside of [ 2.298151] allocated 80-byte region [ffff888008d9b000, ffff888008d9b050) [ 2.299408] [ 2.299601] The buggy address belongs to the physical page: [ 2.300191] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x8d9b [ 2.301001] flags: 0x100000000000000(node=0|zone=1) [ 2.301535] page_type: f5(slab) [ 2.301884] raw: 0100000000000000 ffff888107e46780 dead000000000122 0000000000000000 [ 2.302687] raw: 0000000000000000 0000000800240024 00000000f5000000 0000000000000000 [ 2.303489] page dumped because: kasan: bad access detected [ 2.304092] [ 2.304276] Memory state around the buggy address: [ 2.304801] ffff888008d9af00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 2.305567] ffff888008d9af80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 2.306340] >ffff888008d9b000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 2.307115] ^ [ 2.307474] ffff888008d9b080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 2.308237] ffff888008d9b100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 2.308997] ================================================================== > > > A specific example of this breaking things is mincore which walks > > an internal cursor data structure a byte at a time on assumption > > that page table entry callbacks are called only once for each > > entry. > > > > Fix the problem by resetting walk->action to ACTION_SUBTREE prior > > to the none check. > > > > The pattern also exists in walk_pud_range() so fix it there too. > > > > This issue was found through AI-based fuzzing. > > > > Fixes: 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault") > > It's good to cc the relevant Author(s). > > > Cc: stable@vger.kernel.org > > We really should tell -stable maintainers (and all other users of > earlier kernels) all about the above things. > > > mm/pagewalk.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > This depends on the above info, but I'd prefer to process the bugfix > promptly and defer consideration of the selftest until the next -rc > cycle. Should I send a v3 with the changelog fixed? (in 24 hours, I guess?) Best regards, Hyunwoo Kim ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] selftests/mm: add stale walk->action race test 2026-08-11 16:18 [PATCH v2 0/2] mm/pagewalk: fix stale walk->action escaping walk_pmd_range() Hyunwoo Kim 2026-08-11 16:18 ` [PATCH v2 1/2] " Hyunwoo Kim @ 2026-08-11 16:18 ` Hyunwoo Kim 2026-08-11 16:33 ` Lorenzo Stoakes (ARM) 2026-08-11 17:11 ` David Hildenbrand (Arm) 1 sibling, 2 replies; 11+ messages in thread From: Hyunwoo Kim @ 2026-08-11 16:18 UTC (permalink / raw) To: akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko Cc: linux-mm, linux-kernel, imv4bel The added pagewalk_race_test maps two PMDs and faults in 2MB of the first one. A second thread then faults in the second PMD and drops it again with MADV_DONTNEED in a loop, while the main thread reads Rss for the mapping from /proc/self/smaps. Clearing the second PMD while smaps_pte_range() runs leaves walk->action erroneously set to ACTION_AGAIN, which causes the PUD walk to be retried, so the first PMD is counted twice and Rss comes out twice as large as what was faulted in. mincore() is the caller named in the fix, but the second walk writes past the length mincore() copies back, so it cannot be seen from userspace there. smaps reports what the callbacks counted, so the duplicate shows up in Rss. A failure can only come from the kernel counting the same page twice, so missing the race is harmless. On an unfixed kernel the test fails after a few hundred reads at most and takes about half a second. Assisted-by: Claude:claude-opus-5 Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com> --- tools/testing/selftests/mm/.gitignore | 1 + tools/testing/selftests/mm/Makefile | 2 + tools/testing/selftests/mm/ksft_pagewalk.sh | 4 + .../testing/selftests/mm/pagewalk_race_test.c | 138 ++++++++++++++++++ tools/testing/selftests/mm/run_vmtests.sh | 2 + tools/testing/selftests/mm/vm_util.h | 1 + 6 files changed, 148 insertions(+) create mode 100755 tools/testing/selftests/mm/ksft_pagewalk.sh create mode 100644 tools/testing/selftests/mm/pagewalk_race_test.c diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore index 9ccd9e1447e66b..92f981f97740fd 100644 --- a/tools/testing/selftests/mm/.gitignore +++ b/tools/testing/selftests/mm/.gitignore @@ -66,3 +66,4 @@ merge prctl_thp_disable rmap folio_split_race_test +pagewalk_race_test diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile index e6df968f0971c8..cde9b22f121d4b 100644 --- a/tools/testing/selftests/mm/Makefile +++ b/tools/testing/selftests/mm/Makefile @@ -105,6 +105,7 @@ TEST_GEN_FILES += guard-regions TEST_GEN_FILES += merge TEST_GEN_FILES += rmap TEST_GEN_FILES += folio_split_race_test +TEST_GEN_FILES += pagewalk_race_test ifneq ($(ARCH),arm64) TEST_GEN_FILES += soft-dirty @@ -163,6 +164,7 @@ TEST_PROGS += ksft_mlock.sh TEST_PROGS += ksft_mmap.sh TEST_PROGS += ksft_mremap.sh TEST_PROGS += ksft_pagemap.sh +TEST_PROGS += ksft_pagewalk.sh TEST_PROGS += ksft_pfnmap.sh TEST_PROGS += ksft_pkey.sh TEST_PROGS += ksft_process_madv.sh diff --git a/tools/testing/selftests/mm/ksft_pagewalk.sh b/tools/testing/selftests/mm/ksft_pagewalk.sh new file mode 100755 index 00000000000000..6f6c3ee1c13ef4 --- /dev/null +++ b/tools/testing/selftests/mm/ksft_pagewalk.sh @@ -0,0 +1,4 @@ +#!/bin/sh -e +# SPDX-License-Identifier: GPL-2.0 + +./run_vmtests.sh -t pagewalk diff --git a/tools/testing/selftests/mm/pagewalk_race_test.c b/tools/testing/selftests/mm/pagewalk_race_test.c new file mode 100644 index 00000000000000..42fd6e75e821ed --- /dev/null +++ b/tools/testing/selftests/mm/pagewalk_race_test.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Regression test for a stale walk->action escaping walk_pmd_range() and + * making walk_pud_range() walk the same range twice. + * + * The mapping is two PMDs inside one PUD. PMD 0 is populated once and left + * alone, PMD 1 is faulted in and dropped again by a second thread. Clearing + * PMD 1 under smaps_pte_range() makes it raise ACTION_AGAIN, and since it is + * the last entry the stale value leaves walk_pmd_range(), so smaps accounts + * PMD 0 twice. A kernel that does not reclaim the emptied page table never + * clears PMD 1 and so never hits the race. + * + * A hit can only come from the kernel counting the same page twice, so the + * test cannot fail spuriously. + */ +#define _GNU_SOURCE + +#include <pthread.h> +#include <stdatomic.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/mman.h> +#include <unistd.h> + +#include "vm_util.h" +#include "kselftest.h" + +#define NR_PMDS 2 +#define NR_ROUNDS 20000 +/* Cap on how much of PMD 0 to fault in, so that a large PMD stays cheap. */ +#define POP_MAX (2 * 1024 * 1024) + +static char *area; +static size_t pmd_size; +static atomic_int stop; + +static void *racer(void *arg) +{ + char *pmd1 = area + pmd_size; + + while (atomic_load_explicit(&stop, memory_order_acquire) == 0) { + /* madvise() below keeps the compiler from lifting this out. */ + *pmd1 = 1; + madvise(pmd1, pmd_size, MADV_DONTNEED); + } + return NULL; +} + +static unsigned long smaps_rss_kb(void) +{ + char buf[1024]; + char *entry; + + entry = __get_smap_entry(area, "Rss:", buf, sizeof(buf)); + if (!entry) + ksft_exit_fail_msg("no Rss: entry for the test mapping\n"); + + return strtoul(entry, NULL, 10); +} + +int main(void) +{ + unsigned long max_rss_kb, rss_kb = 0; + size_t size, pop_size, i; + pthread_t thread; + char *raw; + + ksft_print_header(); + + pmd_size = read_pmd_pagesize(); + if (!pmd_size) + ksft_exit_skip("Cannot determine PMD size\n"); + + if (sysconf(_SC_NPROCESSORS_ONLN) < 2) + ksft_exit_skip("Need at least 2 CPUs to race\n"); + + size = NR_PMDS * pmd_size; + + /* + * Align to the mapping size to stay inside one PUD, then trim the + * slack so that smaps has exactly one VMA to report. + */ + raw = mmap(NULL, 2 * size, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0); + if (raw == MAP_FAILED) + ksft_exit_fail_msg("mmap failed\n"); + + area = (char *)(((unsigned long)raw + size - 1) & ~(size - 1)); + if (area != raw) + munmap(raw, area - raw); + if (raw + 2 * size != area + size) + munmap(area + size, raw + 2 * size - (area + size)); + + /* A huge PMD never reaches pte_offset_map_lock(), so keep them out. */ + if (madvise(area, size, MADV_NOHUGEPAGE)) + ksft_exit_skip("MADV_NOHUGEPAGE failed\n"); + + pop_size = pmd_size < POP_MAX ? pmd_size : POP_MAX; + memset(area, 1, pop_size); + + max_rss_kb = (pop_size >> 10) + 256; + + /* Over the limit before racing means this is not our own mapping. */ + rss_kb = smaps_rss_kb(); + if (rss_kb > max_rss_kb) + ksft_exit_fail_msg("Rss is %lu kB before racing, expected at most %lu kB\n", + rss_kb, max_rss_kb); + + ksft_set_plan(1); + ksft_print_msg("racing smaps against MADV_DONTNEED, %d rounds\n", + NR_ROUNDS); + + if (pthread_create(&thread, NULL, racer, NULL)) + ksft_exit_fail_msg("pthread_create failed\n"); + + for (i = 0; i < NR_ROUNDS; i++) { + rss_kb = smaps_rss_kb(); + if (rss_kb > max_rss_kb) + break; + } + + atomic_store_explicit(&stop, 1, memory_order_release); + pthread_join(thread, NULL); + + if (i < NR_ROUNDS) { + ksft_print_msg("walk ran twice over the same range\n"); + ksft_test_result_fail("Rss %lu kB exceeds %lu kB, round %zu\n", + rss_kb, max_rss_kb, i); + } else { + ksft_test_result_pass("Rss within %lu kB over %d rounds\n", + max_rss_kb, NR_ROUNDS); + } + + ksft_exit(i == NR_ROUNDS); + + return 0; +} diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh index 8c296dedf0474d..d90c6370814f7a 100755 --- a/tools/testing/selftests/mm/run_vmtests.sh +++ b/tools/testing/selftests/mm/run_vmtests.sh @@ -398,6 +398,8 @@ fi CATEGORY="pagemap" run_test ./pagemap_ioctl +CATEGORY="pagewalk" run_test ./pagewalk_race_test + CATEGORY="pfnmap" run_test ./pfnmap # COW tests diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index ea8fc8fdf0eb0b..62292e2417d162 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -88,6 +88,7 @@ bool pagemap_is_populated(int fd, char *start); unsigned long pagemap_get_pfn(int fd, char *start); void clear_softdirty(void); bool check_for_pattern(FILE *fp, const char *pattern, char *buf, size_t len); +char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len); uint64_t read_pmd_pagesize(void); unsigned long rss_anon(void); bool check_huge_anon(void *addr, int nr_hpages, uint64_t hpage_size); -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] selftests/mm: add stale walk->action race test 2026-08-11 16:18 ` [PATCH v2 2/2] selftests/mm: add stale walk->action race test Hyunwoo Kim @ 2026-08-11 16:33 ` Lorenzo Stoakes (ARM) 2026-08-11 17:11 ` David Hildenbrand (Arm) 1 sibling, 0 replies; 11+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-08-11 16:33 UTC (permalink / raw) To: Hyunwoo Kim Cc: akpm, david, liam, vbabka, rppt, surenb, mhocko, linux-mm, linux-kernel On Wed, Aug 12, 2026 at 01:18:58AM +0900, Hyunwoo Kim wrote: > The added pagewalk_race_test maps two PMDs and faults in 2MB of the first > one. A second thread then faults in the second PMD and drops it again with > MADV_DONTNEED in a loop, while the main thread reads Rss for the mapping > from /proc/self/smaps. > > Clearing the second PMD while smaps_pte_range() runs leaves walk->action > erroneously set to ACTION_AGAIN, which causes the PUD walk to be retried, > so the first PMD is counted twice and Rss comes out twice as large as what > was faulted in. > > mincore() is the caller named in the fix, but the second walk writes past > the length mincore() copies back, so it cannot be seen from userspace > there. smaps reports what the callbacks counted, so the duplicate shows up > in Rss. > > A failure can only come from the kernel counting the same page twice, so > missing the race is harmless. On an unfixed kernel the test fails after a > few hundred reads at most and takes about half a second. > > Assisted-by: Claude:claude-opus-5 > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com> > --- > tools/testing/selftests/mm/.gitignore | 1 + > tools/testing/selftests/mm/Makefile | 2 + > tools/testing/selftests/mm/ksft_pagewalk.sh | 4 + > .../testing/selftests/mm/pagewalk_race_test.c | 138 ++++++++++++++++++ > tools/testing/selftests/mm/run_vmtests.sh | 2 + > tools/testing/selftests/mm/vm_util.h | 1 + > 6 files changed, 148 insertions(+) > create mode 100755 tools/testing/selftests/mm/ksft_pagewalk.sh > create mode 100644 tools/testing/selftests/mm/pagewalk_race_test.c > > diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore > index 9ccd9e1447e66b..92f981f97740fd 100644 > --- a/tools/testing/selftests/mm/.gitignore > +++ b/tools/testing/selftests/mm/.gitignore > @@ -66,3 +66,4 @@ merge > prctl_thp_disable > rmap > folio_split_race_test > +pagewalk_race_test > diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile > index e6df968f0971c8..cde9b22f121d4b 100644 > --- a/tools/testing/selftests/mm/Makefile > +++ b/tools/testing/selftests/mm/Makefile > @@ -105,6 +105,7 @@ TEST_GEN_FILES += guard-regions > TEST_GEN_FILES += merge > TEST_GEN_FILES += rmap > TEST_GEN_FILES += folio_split_race_test > +TEST_GEN_FILES += pagewalk_race_test > > ifneq ($(ARCH),arm64) > TEST_GEN_FILES += soft-dirty > @@ -163,6 +164,7 @@ TEST_PROGS += ksft_mlock.sh > TEST_PROGS += ksft_mmap.sh > TEST_PROGS += ksft_mremap.sh > TEST_PROGS += ksft_pagemap.sh > +TEST_PROGS += ksft_pagewalk.sh > TEST_PROGS += ksft_pfnmap.sh > TEST_PROGS += ksft_pkey.sh > TEST_PROGS += ksft_process_madv.sh > diff --git a/tools/testing/selftests/mm/ksft_pagewalk.sh b/tools/testing/selftests/mm/ksft_pagewalk.sh > new file mode 100755 > index 00000000000000..6f6c3ee1c13ef4 > --- /dev/null > +++ b/tools/testing/selftests/mm/ksft_pagewalk.sh > @@ -0,0 +1,4 @@ > +#!/bin/sh -e > +# SPDX-License-Identifier: GPL-2.0 > + > +./run_vmtests.sh -t pagewalk > diff --git a/tools/testing/selftests/mm/pagewalk_race_test.c b/tools/testing/selftests/mm/pagewalk_race_test.c > new file mode 100644 > index 00000000000000..42fd6e75e821ed > --- /dev/null > +++ b/tools/testing/selftests/mm/pagewalk_race_test.c > @@ -0,0 +1,138 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Regression test for a stale walk->action escaping walk_pmd_range() and > + * making walk_pud_range() walk the same range twice. > + * > + * The mapping is two PMDs inside one PUD. PMD 0 is populated once and left > + * alone, PMD 1 is faulted in and dropped again by a second thread. Clearing > + * PMD 1 under smaps_pte_range() makes it raise ACTION_AGAIN, and since it is > + * the last entry the stale value leaves walk_pmd_range(), so smaps accounts > + * PMD 0 twice. A kernel that does not reclaim the emptied page table never > + * clears PMD 1 and so never hits the race. > + * > + * A hit can only come from the kernel counting the same page twice, so the > + * test cannot fail spuriously. > + */ > +#define _GNU_SOURCE > + > +#include <pthread.h> > +#include <stdatomic.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <sys/mman.h> > +#include <unistd.h> > + > +#include "vm_util.h" > +#include "kselftest.h" > + > +#define NR_PMDS 2 > +#define NR_ROUNDS 20000 I did say please don't let this be slow :) if the test takes longer than say 200ms then just let it be less reliably repro maybe? > +/* Cap on how much of PMD 0 to fault in, so that a large PMD stays cheap. */ > +#define POP_MAX (2 * 1024 * 1024) > + > +static char *area; > +static size_t pmd_size; > +static atomic_int stop; > + > +static void *racer(void *arg) > +{ > + char *pmd1 = area + pmd_size; > + > + while (atomic_load_explicit(&stop, memory_order_acquire) == 0) { > + /* madvise() below keeps the compiler from lifting this out. */ Err what? :) this seems crazy? You are using an madvise to prevent a compiler optinisation how? :) isn't atomic_load_explicit() a compiler barrier anyway? I think you can probably do this better... see FORCE_READ() for use of volatile to achieve the same thing for a read. But why would zapping a PMD achieve anything for the compiler? So confused > + *pmd1 = 1; > + madvise(pmd1, pmd_size, MADV_DONTNEED); > + } > + return NULL; > +} > + > +static unsigned long smaps_rss_kb(void) > +{ > + char buf[1024]; > + char *entry; > + > + entry = __get_smap_entry(area, "Rss:", buf, sizeof(buf)); > + if (!entry) > + ksft_exit_fail_msg("no Rss: entry for the test mapping\n"); > + > + return strtoul(entry, NULL, 10); > +} > + > +int main(void) > +{ > + unsigned long max_rss_kb, rss_kb = 0; > + size_t size, pop_size, i; > + pthread_t thread; > + char *raw; > + > + ksft_print_header(); > + > + pmd_size = read_pmd_pagesize(); > + if (!pmd_size) > + ksft_exit_skip("Cannot determine PMD size\n"); > + > + if (sysconf(_SC_NPROCESSORS_ONLN) < 2) > + ksft_exit_skip("Need at least 2 CPUs to race\n"); > + > + size = NR_PMDS * pmd_size; > + > + /* > + * Align to the mapping size to stay inside one PUD, then trim the > + * slack so that smaps has exactly one VMA to report. > + */ > + raw = mmap(NULL, 2 * size, PROT_READ | PROT_WRITE, > + MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0); Why MAP_NORESERVE? > + if (raw == MAP_FAILED) > + ksft_exit_fail_msg("mmap failed\n"); > + > + area = (char *)(((unsigned long)raw + size - 1) & ~(size - 1)); This is horrible, break it out into a sensible helper. Magical bitwise stuff all compressed into one is not something we want. > + if (area != raw) > + munmap(raw, area - raw); > + if (raw + 2 * size != area + size) > + munmap(area + size, raw + 2 * size - (area + size)); Again you're compressing things too much. Keep it simple. > + > + /* A huge PMD never reaches pte_offset_map_lock(), so keep them out. */ > + if (madvise(area, size, MADV_NOHUGEPAGE)) > + ksft_exit_skip("MADV_NOHUGEPAGE failed\n"); No idea why you'd skip on this. This is another reason why you should use the kselftest harness, ASSERT_EQ(..., 0) is much easier... and it has sensible skipping stuff too. > + > + pop_size = pmd_size < POP_MAX ? pmd_size : POP_MAX; > + memset(area, 1, pop_size); > + > + max_rss_kb = (pop_size >> 10) + 256; > + > + /* Over the limit before racing means this is not our own mapping. */ > + rss_kb = smaps_rss_kb(); > + if (rss_kb > max_rss_kb) > + ksft_exit_fail_msg("Rss is %lu kB before racing, expected at most %lu kB\n", > + rss_kb, max_rss_kb); > + > + ksft_set_plan(1); > + ksft_print_msg("racing smaps against MADV_DONTNEED, %d rounds\n", > + NR_ROUNDS); > + > + if (pthread_create(&thread, NULL, racer, NULL)) > + ksft_exit_fail_msg("pthread_create failed\n"); > + > + for (i = 0; i < NR_ROUNDS; i++) { > + rss_kb = smaps_rss_kb(); > + if (rss_kb > max_rss_kb) > + break; > + } > + > + atomic_store_explicit(&stop, 1, memory_order_release); > + pthread_join(thread, NULL); > + > + if (i < NR_ROUNDS) { > + ksft_print_msg("walk ran twice over the same range\n"); > + ksft_test_result_fail("Rss %lu kB exceeds %lu kB, round %zu\n", > + rss_kb, max_rss_kb, i); > + } else { > + ksft_test_result_pass("Rss within %lu kB over %d rounds\n", > + max_rss_kb, NR_ROUNDS); > + } > + > + ksft_exit(i == NR_ROUNDS); > + > + return 0; Putting this all into one function is disgusting. It might be test code but that's new excuse for being schloppy. Separate things out into functions please and.... > +} ...use the kselftest_harness, see guard_regions.c for an example of how it's used. I know it's just 1 test but it avoids all the stupid plan and manual ksft_test_*() invocations and gets you ASSERT_*() etc. > diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh > index 8c296dedf0474d..d90c6370814f7a 100755 > --- a/tools/testing/selftests/mm/run_vmtests.sh > +++ b/tools/testing/selftests/mm/run_vmtests.sh > @@ -398,6 +398,8 @@ fi > > CATEGORY="pagemap" run_test ./pagemap_ioctl > > +CATEGORY="pagewalk" run_test ./pagewalk_race_test > + > CATEGORY="pfnmap" run_test ./pfnmap > > # COW tests > diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h > index ea8fc8fdf0eb0b..62292e2417d162 100644 > --- a/tools/testing/selftests/mm/vm_util.h > +++ b/tools/testing/selftests/mm/vm_util.h > @@ -88,6 +88,7 @@ bool pagemap_is_populated(int fd, char *start); > unsigned long pagemap_get_pfn(int fd, char *start); > void clear_softdirty(void); > bool check_for_pattern(FILE *fp, const char *pattern, char *buf, size_t len); > +char *__get_smap_entry(void *addr, const char *pattern, char *buf, size_t len); > uint64_t read_pmd_pagesize(void); > unsigned long rss_anon(void); > bool check_huge_anon(void *addr, int nr_hpages, uint64_t hpage_size); > -- > 2.43.0 > -- Cheers, Lorenzo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] selftests/mm: add stale walk->action race test 2026-08-11 16:18 ` [PATCH v2 2/2] selftests/mm: add stale walk->action race test Hyunwoo Kim 2026-08-11 16:33 ` Lorenzo Stoakes (ARM) @ 2026-08-11 17:11 ` David Hildenbrand (Arm) 2026-08-11 18:23 ` Lorenzo Stoakes (ARM) 1 sibling, 1 reply; 11+ messages in thread From: David Hildenbrand (Arm) @ 2026-08-11 17:11 UTC (permalink / raw) To: Hyunwoo Kim, akpm, ljs, liam, vbabka, rppt, surenb, mhocko Cc: linux-mm, linux-kernel On 8/11/26 18:18, Hyunwoo Kim wrote: > The added pagewalk_race_test maps two PMDs and faults in 2MB of the first > one. A second thread then faults in the second PMD and drops it again with > MADV_DONTNEED in a loop, while the main thread reads Rss for the mapping > from /proc/self/smaps. > > Clearing the second PMD while smaps_pte_range() runs leaves walk->action > erroneously set to ACTION_AGAIN, which causes the PUD walk to be retried, > so the first PMD is counted twice and Rss comes out twice as large as what > was faulted in. > > mincore() is the caller named in the fix, but the second walk writes past > the length mincore() copies back, so it cannot be seen from userspace > there. smaps reports what the callbacks counted, so the duplicate shows up > in Rss. > > A failure can only come from the kernel counting the same page twice, so > missing the race is harmless. On an unfixed kernel the test fails after a > few hundred reads at most and takes about half a second. > > Assisted-by: Claude:claude-opus-5 > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com> > --- I'm sorry, but I don't consider such a specialized reproducer for a problem we hit once a good fit for a selftest. Or am I missing something, that this here is more generic? -- Cheers, David ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] selftests/mm: add stale walk->action race test 2026-08-11 17:11 ` David Hildenbrand (Arm) @ 2026-08-11 18:23 ` Lorenzo Stoakes (ARM) 2026-08-11 18:49 ` David Hildenbrand (Arm) 0 siblings, 1 reply; 11+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-08-11 18:23 UTC (permalink / raw) To: David Hildenbrand (Arm) Cc: Hyunwoo Kim, akpm, liam, vbabka, rppt, surenb, mhocko, linux-mm, linux-kernel On Tue, Aug 11, 2026 at 07:11:59PM +0200, David Hildenbrand (Arm) wrote: > On 8/11/26 18:18, Hyunwoo Kim wrote: > > The added pagewalk_race_test maps two PMDs and faults in 2MB of the first > > one. A second thread then faults in the second PMD and drops it again with > > MADV_DONTNEED in a loop, while the main thread reads Rss for the mapping > > from /proc/self/smaps. > > > > Clearing the second PMD while smaps_pte_range() runs leaves walk->action > > erroneously set to ACTION_AGAIN, which causes the PUD walk to be retried, > > so the first PMD is counted twice and Rss comes out twice as large as what > > was faulted in. > > > > mincore() is the caller named in the fix, but the second walk writes past > > the length mincore() copies back, so it cannot be seen from userspace > > there. smaps reports what the callbacks counted, so the duplicate shows up > > in Rss. > > > > A failure can only come from the kernel counting the same page twice, so > > missing the race is harmless. On an unfixed kernel the test fails after a > > few hundred reads at most and takes about half a second. > > > > Assisted-by: Claude:claude-opus-5 > > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com> > > --- > > I'm sorry, but I don't consider such a specialized reproducer for a problem we > hit once a good fit for a selftest. > > Or am I missing something, that this here is more generic? To be fair I did ask him for this. :) I thought it would be useful to have a regression test because there's certainly no harm in it? The code is very sensitive and catching a similar mistake in future could be useful. If you're sure this isn't valuable then he could just drop it, but it's my fault sorry Hyunwoo for giving you extra work in this case! My bad. > > -- > Cheers, > > David -- Cheers, Lorenzo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] selftests/mm: add stale walk->action race test 2026-08-11 18:23 ` Lorenzo Stoakes (ARM) @ 2026-08-11 18:49 ` David Hildenbrand (Arm) 0 siblings, 0 replies; 11+ messages in thread From: David Hildenbrand (Arm) @ 2026-08-11 18:49 UTC (permalink / raw) To: Lorenzo Stoakes (ARM) Cc: Hyunwoo Kim, akpm, liam, vbabka, rppt, surenb, mhocko, linux-mm, linux-kernel On 8/11/26 20:23, Lorenzo Stoakes (ARM) wrote: > On Tue, Aug 11, 2026 at 07:11:59PM +0200, David Hildenbrand (Arm) wrote: >> On 8/11/26 18:18, Hyunwoo Kim wrote: >>> The added pagewalk_race_test maps two PMDs and faults in 2MB of the first >>> one. A second thread then faults in the second PMD and drops it again with >>> MADV_DONTNEED in a loop, while the main thread reads Rss for the mapping >>> from /proc/self/smaps. >>> >>> Clearing the second PMD while smaps_pte_range() runs leaves walk->action >>> erroneously set to ACTION_AGAIN, which causes the PUD walk to be retried, >>> so the first PMD is counted twice and Rss comes out twice as large as what >>> was faulted in. >>> >>> mincore() is the caller named in the fix, but the second walk writes past >>> the length mincore() copies back, so it cannot be seen from userspace >>> there. smaps reports what the callbacks counted, so the duplicate shows up >>> in Rss. >>> >>> A failure can only come from the kernel counting the same page twice, so >>> missing the race is harmless. On an unfixed kernel the test fails after a >>> few hundred reads at most and takes about half a second. >>> >>> Assisted-by: Claude:claude-opus-5 >>> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com> >>> --- >> >> I'm sorry, but I don't consider such a specialized reproducer for a problem we >> hit once a good fit for a selftest. >> >> Or am I missing something, that this here is more generic? > > To be fair I did ask him for this. :) > > I thought it would be useful to have a regression test because there's certainly > no harm in it? The code is very sensitive and catching a similar mistake in > future could be useful. > > If you're sure this isn't valuable then he could just drop it, but it's my fault > sorry Hyunwoo for giving you extra work in this case! My bad. pagewalk_race_test is just rather odd. If this would be part of a bigger test that covers more things than just one odd race, it might be better. But we certainly don't want one new test file for each kernel bug we ever trigger, right? :) -- Cheers, David ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-08-11 20:17 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-11 16:18 [PATCH v2 0/2] mm/pagewalk: fix stale walk->action escaping walk_pmd_range() Hyunwoo Kim 2026-08-11 16:18 ` [PATCH v2 1/2] " Hyunwoo Kim 2026-08-11 16:23 ` Lorenzo Stoakes (ARM) 2026-08-11 17:10 ` David Hildenbrand (Arm) 2026-08-11 19:37 ` Andrew Morton 2026-08-11 20:17 ` Hyunwoo Kim 2026-08-11 16:18 ` [PATCH v2 2/2] selftests/mm: add stale walk->action race test Hyunwoo Kim 2026-08-11 16:33 ` Lorenzo Stoakes (ARM) 2026-08-11 17:11 ` David Hildenbrand (Arm) 2026-08-11 18:23 ` Lorenzo Stoakes (ARM) 2026-08-11 18:49 ` David Hildenbrand (Arm)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox