linux-kernel - Re: [PATCH v1 2/7] mm: make zap_pte_range() handle full within-PMD range

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAG48ez28bTO2KChkzktf5XJGDsC81mMhKaeny4vmoOMJ38wJ4A@mail.gmail.com>
Date: Thu, 17 Oct 2024 20:06:57 +0200
From: Jann Horn <jannh@...gle.com>
To: Qi Zheng <zhengqi.arch@...edance.com>
Cc: david@...hat.com, hughd@...gle.com, willy@...radead.org, mgorman@...e.de, 
	muchun.song@...ux.dev, vbabka@...nel.org, akpm@...ux-foundation.org, 
	zokeefe@...gle.com, rientjes@...gle.com, peterx@...hat.com, 
	linux-mm@...ck.org, linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH v1 2/7] mm: make zap_pte_range() handle full within-PMD range

On Thu, Oct 17, 2024 at 11:48 AM Qi Zheng <zhengqi.arch@...edance.com> wrote:
> In preparation for reclaiming empty PTE pages, this commit first makes
> zap_pte_range() to handle the full within-PMD range, so that we can more
> easily detect and free PTE pages in this function in subsequent commits.

I think your patch causes some unintended difference in behavior:

> Signed-off-by: Qi Zheng <zhengqi.arch@...edance.com>
> ---
>  mm/memory.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index caa6ed0a7fe5b..fd57c0f49fce2 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1602,6 +1602,7 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
>         swp_entry_t entry;
>         int nr;
>
> +retry:

This "retry" label is below the line "bool force_flush = false,
force_break = false;", so I think after force_break is set once and
you go through the retry path, every subsequent present PTE will again
bail out and retry. I think that doesn't lead to anything bad, but it
seems unintended.