[PATCH] mm/pagewalk: fix stale walk->action escaping walk_pmd_range()

[PATCH] mm/pagewalk: fix stale walk->action escaping walk_pmd_range()

[Hyunwoo Kim]

walk_pmd_range() resets walk->action in only one place in its loop body,
and that place is after the pmd_none() branch. For a walker with no
->install_pte, that branch continues to the next entry without passing the
reset. So if ->pmd_entry() sets ACTION_AGAIN and returns 0, and the PMD has
become none by the time the loop restarts at the again label, the reset is
skipped. If the remaining entries are all none too, the loop returns 0 with
ACTION_AGAIN still set. The ACTION_AGAIN that walk_pte_range() sets when
pte_offset_map_lock() fails escapes the same way.

Nothing looked at that value after walk_pmd_range() returned until commit
3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault")
turned that into a problem. It added both the PUD check that sets
ACTION_AGAIN before the loop is entered and the test in walk_pud_range()
that picks the value up right after walk_pmd_range() returns and walks
[addr, pud_addr_end(addr, end)) again. That is fine for the PUD check,
since none of walk_pmd_range()'s own callbacks have run at that point, but
a value that escaped as described above arrives after those callbacks have
already covered the range.

For mincore(2) this becomes an out-of-bounds write. ->pmd_entry() and
->pte_hole() advance the walk->private cursor by one byte per page, the
buffer is a single page from __get_free_page(), and mincore(2) asks for at
most PAGE_SIZE entries at a time, so there is no room to spare. Walking
the range a second time pushes the cursor past the end of the buffer, and
it does so again every time the race is hit. Reproducing this needs no
privileges: run mincore(2) over a 16 MiB anonymous mapping marked
MADV_NOHUGEPAGE while another thread repeatedly faults in a PMD-aligned
2 MiB range inside it and then drops it with madvise(MADV_DONTNEED).

Move the reset to the first statement of the loop body. walk_pud_range()
has the same shape and gets the same change; walk_p4d_range() never looks
at walk->action, so that hunk keeps the two functions in sync rather than
fixing a second bug.

Fixes: 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault")
Cc: stable@vger.kernel.org
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

Re: [PATCH] mm/pagewalk: fix stale walk->action escaping walk_pmd_range()

[Lorenzo Stoakes (ARM)]

Since it's 2026 + this is your first patch in mm AFAICT, and it's also a
very fiddly and specific issue, I do have to ask - was there was any AI
involved in making this?

If so you should add an Assisted-by: tag as per
https://docs.kernel.org/process/coding-assistants.html please :)

On Mon, Aug 10, 2026 at 06:45:17PM +0900, Hyunwoo Kim wrote:
> walk_pmd_range() resets walk->action in only one place in its loop body,
> and that place is after the pmd_none() branch. For a walker with no

Newline after full stop please, this is too many words in one big block.

What is a 'reset'? This doesn't really mean anything. You mean sets
walk->action = ACTION_SUBTREE, which is the default action.

Your commit message doesn't mention ACTION_SUBTREE anywhere, it should.

Also what you're saying here is just untrue if you consider reset to be
assignment to walk->action (which is a reasonable interpretation) -
walk_pmd_range() changes walk->action in _two_ places.

Just be clear that you by reset you mean assigning walk->action =
ACTION_SUBTREE.

> ->install_pte, that branch continues to the next entry without passing the
> reset. So if ->pmd_entry() sets ACTION_AGAIN and returns 0, and the PMD has

Not passing what reset? This is so unclear.

> become none by the time the loop restarts at the again label, the reset is
> skipped. If the remaining entries are all none too, the loop returns 0 with
> ACTION_AGAIN still set. The ACTION_AGAIN that walk_pte_range() sets when
> pte_offset_map_lock() fails escapes the same way.

OK so what you mean to say is:

	if (ops->pmd_entry)
		err = ops->pmd_entry(pmd, addr, next, walk); <- 1. sets walk->action =
								   ACTION_AGAIN
		if (err)
			break;

		if (walk->action == ACTION_AGAIN)
			goto again;

Then above that code:

again:
		next = pmd_addr_end(addr, end);
		if (pmd_none(*pmd)) {   <- 2. This triggers because PMD became empty
			if (has_install)
				err = __pte_alloc(walk->mm, pmd);
			else if (ops->pte_hole)
				err = ops->pte_hole(addr, next, depth, walk);
			if (err)
				break;
			if (!has_install)
				continue; <- 3. Loop around to the next entry, with
						walk->action erroneously set to
						ACTION_AGAIN still.
		}

		walk->action = ACTION_SUBTREE; <- 4. This would reset it EXCEPT if
		                                     you are at the end of the range.

Then in walk_pud_range():

		err = walk_pmd_range(pud, addr, next, walk);
		if (err)
			break;

		if (walk->action == ACTION_AGAIN) <- 5. Uh oh doing a retry for no
		                                        reason.
			goto again;

So it's only if you're at the end of the PMD range that it's a problem
right? You should say that.

The user-visible problem here is that you can end up calling the callbacks
too many times which explicitly breaks mincore.

Given I had to decode this it does make me wonder if you fully understand
this or if it's just unclear language. This sort of word salad is very
LLM-ish :)

Anyway overall maybe rewrite the commit message to something like:

	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_entry() so fix it there too.


>
> Nothing looked at that value after walk_pmd_range() returned until commit
> 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault")
> turned that into a problem. It added both the PUD check that sets
> ACTION_AGAIN before the loop is entered and the test in walk_pud_range()
> that picks the value up right after walk_pmd_range() returns and walks
> [addr, pud_addr_end(addr, end)) again. That is fine for the PUD check,
> since none of walk_pmd_range()'s own callbacks have run at that point, but
> a value that escaped as described above arrives after those callbacks have
> already covered the range.
>
> For mincore(2) this becomes an out-of-bounds write. ->pmd_entry() and
> ->pte_hole() advance the walk->private cursor by one byte per page, the
> buffer is a single page from __get_free_page(), and mincore(2) asks for at
> most PAGE_SIZE entries at a time, so there is no room to spare. Walking
> the range a second time pushes the cursor past the end of the buffer, and
> it does so again every time the race is hit. Reproducing this needs no
> privileges: run mincore(2) over a 16 MiB anonymous mapping marked
> MADV_NOHUGEPAGE while another thread repeatedly faults in a PMD-aligned
> 2 MiB range inside it and then drops it with madvise(MADV_DONTNEED).

Obviouisly see above on commit message.

Is it possible to add a self test that does something like this or is it too
racey to be practical?

>
> Move the reset to the first statement of the loop body. walk_pud_range()
> has the same shape and gets the same change; walk_p4d_range() never looks
> at walk->action, so that hunk keeps the two functions in sync rather than
> fixing a second bug.

Your commit message doesn't mention how you discovered this. If it was AI
suggesting it (whether locally or sashiko in reply to some review) you
should reference it.

If it was simply hardcore code inspection then you should say so too :)

>
> Fixes: 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault")
> Cc: stable@vger.kernel.org

This all seems right.

> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>

The fix itself looks right, but you need to address the other feedback and
respin (also please send a respin with at least 1 day's delay).

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

Re: [PATCH] mm/pagewalk: fix stale walk->action escaping walk_pmd_range()

[Hyunwoo Kim]

On Mon, Aug 10, 2026 at 12:19:54PM +0100, Lorenzo Stoakes (ARM) wrote:
> Since it's 2026 + this is your first patch in mm AFAICT, 

Yeah, my home town is netdev :)

> and it's also a
> very fiddly and specific issue, I do have to ask - was there was any AI
> involved in making this?
> 
> If so you should add an Assisted-by: tag as per
> https://docs.kernel.org/process/coding-assistants.html please :)
> 
> On Mon, Aug 10, 2026 at 06:45:17PM +0900, Hyunwoo Kim wrote:
> > walk_pmd_range() resets walk->action in only one place in its loop body,
> > and that place is after the pmd_none() branch. For a walker with no
> 
> Newline after full stop please, this is too many words in one big block.
> 
> What is a 'reset'? This doesn't really mean anything. You mean sets
> walk->action = ACTION_SUBTREE, which is the default action.
> 
> Your commit message doesn't mention ACTION_SUBTREE anywhere, it should.
> 
> Also what you're saying here is just untrue if you consider reset to be
> assignment to walk->action (which is a reasonable interpretation) -
> walk_pmd_range() changes walk->action in _two_ places.
> 
> Just be clear that you by reset you mean assigning walk->action =
> ACTION_SUBTREE.
> 
> > ->install_pte, that branch continues to the next entry without passing the
> > reset. So if ->pmd_entry() sets ACTION_AGAIN and returns 0, and the PMD has
> 
> Not passing what reset? This is so unclear.
> 
> > become none by the time the loop restarts at the again label, the reset is
> > skipped. If the remaining entries are all none too, the loop returns 0 with
> > ACTION_AGAIN still set. The ACTION_AGAIN that walk_pte_range() sets when
> > pte_offset_map_lock() fails escapes the same way.
> 
> OK so what you mean to say is:
> 
> 	if (ops->pmd_entry)
> 		err = ops->pmd_entry(pmd, addr, next, walk); <- 1. sets walk->action =
> 								   ACTION_AGAIN
> 		if (err)
> 			break;
> 
> 		if (walk->action == ACTION_AGAIN)
> 			goto again;
> 
> Then above that code:
> 
> again:
> 		next = pmd_addr_end(addr, end);
> 		if (pmd_none(*pmd)) {   <- 2. This triggers because PMD became empty
> 			if (has_install)
> 				err = __pte_alloc(walk->mm, pmd);
> 			else if (ops->pte_hole)
> 				err = ops->pte_hole(addr, next, depth, walk);
> 			if (err)
> 				break;
> 			if (!has_install)
> 				continue; <- 3. Loop around to the next entry, with
> 						walk->action erroneously set to
> 						ACTION_AGAIN still.
> 		}
> 
> 		walk->action = ACTION_SUBTREE; <- 4. This would reset it EXCEPT if
> 		                                     you are at the end of the range.
> 
> Then in walk_pud_range():
> 
> 		err = walk_pmd_range(pud, addr, next, walk);
> 		if (err)
> 			break;
> 
> 		if (walk->action == ACTION_AGAIN) <- 5. Uh oh doing a retry for no
> 		                                        reason.
> 			goto again;
> 
> So it's only if you're at the end of the PMD range that it's a problem
> right? 

Yes, or more precisely when nothing after it sets walk->action = ACTION_SUBTREE.

> You should say that.
> 
> The user-visible problem here is that you can end up calling the callbacks
> too many times which explicitly breaks mincore.
> 
> Given I had to decode this it does make me wonder if you fully understand
> this or if it's just unclear language. This sort of word salad is very
> LLM-ish :)

The commit message was written with help from the lovely Claude. Not the
most readable, admittedly :)

> 
> Anyway overall maybe rewrite the commit message to something like:
> 
> 	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_entry() so fix it there too.
> 
> 
> >
> > Nothing looked at that value after walk_pmd_range() returned until commit
> > 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault")
> > turned that into a problem. It added both the PUD check that sets
> > ACTION_AGAIN before the loop is entered and the test in walk_pud_range()
> > that picks the value up right after walk_pmd_range() returns and walks
> > [addr, pud_addr_end(addr, end)) again. That is fine for the PUD check,
> > since none of walk_pmd_range()'s own callbacks have run at that point, but
> > a value that escaped as described above arrives after those callbacks have
> > already covered the range.
> >
> > For mincore(2) this becomes an out-of-bounds write. ->pmd_entry() and
> > ->pte_hole() advance the walk->private cursor by one byte per page, the
> > buffer is a single page from __get_free_page(), and mincore(2) asks for at
> > most PAGE_SIZE entries at a time, so there is no room to spare. Walking
> > the range a second time pushes the cursor past the end of the buffer, and
> > it does so again every time the race is hit. Reproducing this needs no
> > privileges: run mincore(2) over a 16 MiB anonymous mapping marked
> > MADV_NOHUGEPAGE while another thread repeatedly faults in a PMD-aligned
> > 2 MiB range inside it and then drops it with madvise(MADV_DONTNEED).
> 
> Obviouisly see above on commit message.
> 
> Is it possible to add a self test that does something like this or is it too
> racey to be practical?

It looks quite race dependent. Triggering it deterministically would need
some of the race window widening tricks from exploit work, which I don't
think belongs in a selftest. I can still write one if you'd like.

> 
> >
> > Move the reset to the first statement of the loop body. walk_pud_range()
> > has the same shape and gets the same change; walk_p4d_range() never looks
> > at walk->action, so that hunk keeps the two functions in sync rather than
> > fixing a second bug.
> 
> Your commit message doesn't mention how you discovered this. If it was AI
> suggesting it (whether locally or sashiko in reply to some review) you
> should reference it.
> 
> If it was simply hardcore code inspection then you should say so too :)

It was found by accident while fuzzing a different subsystem. Either way,
the fuzzer itself was written by AI, so,

Assisted-by: Claude:claude-opus-5

> 
> >
> > Fixes: 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault")
> > Cc: stable@vger.kernel.org
> 
> This all seems right.
> 
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> 
> The fix itself looks right, but you need to address the other feedback and
> respin (also please send a respin with at least 1 day's delay).

OK, will do.


Best regards,
Hyunwoo Kim

Re: [PATCH] mm/pagewalk: fix stale walk->action escaping walk_pmd_range()

[Lorenzo Stoakes (ARM)]

On Tue, Aug 11, 2026 at 12:50:17AM +0900, Hyunwoo Kim wrote:
> On Mon, Aug 10, 2026 at 12:19:54PM +0100, Lorenzo Stoakes (ARM) wrote:
> > Since it's 2026 + this is your first patch in mm AFAICT,
>
> Yeah, my home town is netdev :)

Ah I see :) well welcome, this is regardless a useful bug fix so it's
appreciated!

> > So it's only if you're at the end of the PMD range that it's a problem
> > right?
>
> Yes, or more precisely when nothing after it sets walk->action = ACTION_SUBTREE.

Yeah I word this a bit better in my proposed commit message :)

>
> > You should say that.
> >
> > The user-visible problem here is that you can end up calling the callbacks
> > too many times which explicitly breaks mincore.
> >
> > Given I had to decode this it does make me wonder if you fully understand
> > this or if it's just unclear language. This sort of word salad is very
> > LLM-ish :)
>
> The commit message was written with help from the lovely Claude. Not the
> most readable, admittedly :)

Yeah I did wonder :)

I'm totally fine with using an LLM to help with language, especially if English
is not your primary language, but obviously it can go... wrong :)

So will always feedback if the commit message is unclear.

LLMs seem to have a habit of constructing word salads that end up effectively
like 'the code in English' which ends up confusing more than helping.

Key thing is keeping things as short and clear as possible.

> > Is it possible to add a self test that does something like this or is it too
> > racey to be practical?
>
> It looks quite race dependent. Triggering it deterministically would need
> some of the race window widening tricks from exploit work, which I don't
> think belongs in a selftest. I can still write one if you'd like.

Yeah that's what I wondered about. It might still be useful even if it's racey,
as long as obviously the _test itself_ isn't flakey :) and also - it can't take
too long to run, either.

It'd be good to have a regression test for this.

>
> >
> > >
> > > Move the reset to the first statement of the loop body. walk_pud_range()
> > > has the same shape and gets the same change; walk_p4d_range() never looks
> > > at walk->action, so that hunk keeps the two functions in sync rather than
> > > fixing a second bug.
> >
> > Your commit message doesn't mention how you discovered this. If it was AI
> > suggesting it (whether locally or sashiko in reply to some review) you
> > should reference it.
> >
> > If it was simply hardcore code inspection then you should say so too :)
>
> It was found by accident while fuzzing a different subsystem. Either way,
> the fuzzer itself was written by AI, so,
>
> Assisted-by: Claude:claude-opus-5

Ack thanks :)

>
> >
> > >
> > > Fixes: 3b89863c3fa4 ("mm/pagewalk: fix race between concurrent split and refault")
> > > Cc: stable@vger.kernel.org
> >
> > This all seems right.
> >
> > > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> >
> > The fix itself looks right, but you need to address the other feedback and
> > respin (also please send a respin with at least 1 day's delay).
>
> OK, will do.

Great thanks!

>
>
> Best regards,
> Hyunwoo Kim

--
Cheers, Lorenzo

Re: [PATCH] mm/pagewalk: fix stale walk->action escaping walk_pmd_range()

[Andrew Morton]

On Mon, 10 Aug 2026 16:55:25 +0100 "Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote:

> > The commit message was written with help from the lovely Claude. Not the
> > most readable, admittedly :)
> 
> Yeah I did wonder :)
> 
> I'm totally fine with using an LLM to help with language, especially if English
> is not your primary language, but obviously it can go... wrong :)

yup.  Changelogs from Asian developers have become a lot easier to read
in recent times.  But please always carefully review the resulting
output.

> So will always feedback if the commit message is unclear.
> 
> LLMs seem to have a habit of constructing word salads that end up effectively
> like 'the code in English' which ends up confusing more than helping.

I wonder if it would help to put the initial changelog text through the
LLM, but don't show it any code.

> Key thing is keeping things as short and clear as possible.

I pasted the original changelog into Gemini asking "make this shorter"
and got

Issue

  walk_pmd_range() only resets walk->action after the pmd_none()
  check.  For a walker without ->install_pte, a pmd_none() branch jumps
  to the next entry before resetting walk->action.  If ACTION_AGAIN is
  set and the entry becomes none, the loop can return 0 while leaving
  ACTION_AGAIN active.

  Since commit 3b89863c3fa4, walk_pud_range() reads this leaked
  ACTION_AGAIN status and rewalks the PMD range.  Because callbacks
  like ->pmd_entry() and ->pte_hole() have already processed the range
  once, re-executing them advances walk->private beyond the allocated
  buffer, causing an unprivileged out-of-bounds write (e.g., via
  mincore(2)).

Fix

  Move the walk->action = ACTION_SUBTREE reset to the top of the loop
  body in walk_pmd_range().  Apply the same update to walk_pud_range()
  and walk_p4d_range() to maintain code consistency.

Which is at least shorter ;)


We have a description of the userspace-visible runtime effects of the
bug, which is good, especially with cc:stable fixes.  If there's a bug
report or a backtrace to Link: to then please do that.