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

[Lorenzo Stoakes (ARM)]

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@xxxxxxxxxxxxxxx
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Hyunwoo Kim <imv4bel@xxxxxxxxx>

Thanks, LGTM so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>

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