linux-kernel - Re: [PATCH] mm/damon/vaddr: do not repeat pte_offset_map

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <07267988-1736-1372-8994-76892c5cc804@google.com>
Date: Mon, 29 Sep 2025 20:18:13 -0700 (PDT)
From: Hugh Dickins <hughd@...gle.com>
To: SeongJae Park <sj@...nel.org>
cc: Andrew Morton <akpm@...ux-foundation.org>, Hugh Dickins <hughd@...gle.com>, 
    damon@...ts.linux.dev, kernel-team@...a.com, linux-kernel@...r.kernel.org, 
    linux-mm@...ck.org, Xinyu Zheng <zhengxinyu6@...wei.com>
Subject: Re: [PATCH] mm/damon/vaddr: do not repeat pte_offset_map_lock()
 until success

On Mon, 29 Sep 2025, SeongJae Park wrote:

> DAMON's virtual address space operation set implementation (vaddr) calls
> pte_offset_map_lock() inside the page table walk callback function.
> This is for reading and writing page table accessed bits.  If
> pte_offset_map_lock() fails, it retries by returning the page table walk
> callback function with ACTION_AGAIN.
> 
> pte_offset_map_lock() can continuously fail if the target is a pmd
> migration entry, though.  Hence it could cause an infinite page table
> walk if the migration cannot be done until the page table walk is
> finished.  This indeed caused a soft lockup when CPU hotplugging and
> DAMON were running in parallel.
> 
> Avoid the infinite loop by simply not retrying the page table walk.
> DAMON is promising only a best-effort accuracy, so missing access to
> such pages is no problem.
> 
> Reported-by: Xinyu Zheng <zhengxinyu6@...wei.com>
> Closes: https://lore.kernel.org/20250918030029.2652607-1-zhengxinyu6@huawei.com
> Fixes: 7780d04046a2 ("mm/pagewalkers: ACTION_AGAIN if pte_offset_map_lock() fails")
> Cc: <stable@...r.kernel.org> # 6.5.x
> Cc: Hugh Dickins <hughd@...gle.com>
> Signed-off-by: SeongJae Park <sj@...nel.org>

Thanks,
Acked-by: Hugh Dickins <hughd@...gle.com>

> ---
>  mm/damon/vaddr.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 8c048f9b129e..7e834467b2d8 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -328,10 +328,8 @@ static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr,
>  	}
>  
>  	pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl);
> -	if (!pte) {
> -		walk->action = ACTION_AGAIN;
> +	if (!pte)
>  		return 0;
> -	}
>  	if (!pte_present(ptep_get(pte)))
>  		goto out;
>  	damon_ptep_mkold(pte, walk->vma, addr);
> @@ -481,10 +479,8 @@ static int damon_young_pmd_entry(pmd_t *pmd, unsigned long addr,
>  #endif	/* CONFIG_TRANSPARENT_HUGEPAGE */
>  
>  	pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl);
> -	if (!pte) {
> -		walk->action = ACTION_AGAIN;
> +	if (!pte)
>  		return 0;
> -	}
>  	ptent = ptep_get(pte);
>  	if (!pte_present(ptent))
>  		goto out;
> 
> base-commit: 3169a901e935bc1f2d2eec0171abcf524b7747e4
> -- 
> 2.39.5