| 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
| ||
|
Message-ID: <48375ad8-7461-446e-9002-8d326fba137b@lucifer.local>
Date: Mon, 9 Jun 2025 11:45:36 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Ryan Roberts <ryan.roberts@....com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Alexandre Ghiti <alex@...ti.fr>, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>, David Hildenbrand <david@...hat.com>,
Rik van Riel <riel@...riel.com>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>, Harry Yoo <harry.yoo@...cle.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-riscv@...ts.infradead.org, linux-mm@...ck.org
Subject: Re: [PATCH v1] mm: Remove arch_flush_tlb_batched_pending() arch
helper
On Mon, Jun 09, 2025 at 11:31:30AM +0100, Ryan Roberts wrote:
> Since commit 4b634918384c ("arm64/mm: Close theoretical race where stale
> TLB entry remains valid"), all arches that use tlbbatch for reclaim
> (arm64, riscv, x86) implement arch_flush_tlb_batched_pending() with a
> flush_tlb_mm().
>
> So let's simplify by removing the unnecessary abstraction and doing the
> flush_tlb_mm() directly in flush_tlb_batched_pending(). This effectively
> reverts commit db6c1f6f236d ("mm/tlbbatch: introduce
> arch_flush_tlb_batched_pending()").
>
> Suggested-by: Will Deacon <will@...nel.org>
> Signed-off-by: Ryan Roberts <ryan.roberts@....com>
Thanks, love to see an arch_*() helper go :)
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Couple points below.
> ---
> arch/arm64/include/asm/tlbflush.h | 11 -----------
> arch/riscv/include/asm/tlbflush.h | 1 -
> arch/riscv/mm/tlbflush.c | 5 -----
> arch/x86/include/asm/tlbflush.h | 5 -----
> mm/rmap.c | 2 +-
> 5 files changed, 1 insertion(+), 23 deletions(-)
>
> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> index aa9efee17277..18a5dc0c9a54 100644
> --- a/arch/arm64/include/asm/tlbflush.h
> +++ b/arch/arm64/include/asm/tlbflush.h
> @@ -322,17 +322,6 @@ static inline bool arch_tlbbatch_should_defer(struct mm_struct *mm)
> return true;
> }
>
> -/*
> - * If mprotect/munmap/etc occurs during TLB batched flushing, we need to ensure
> - * all the previously issued TLBIs targeting mm have completed. But since we
> - * can be executing on a remote CPU, a DSB cannot guarantee this like it can
> - * for arch_tlbbatch_flush(). Our only option is to flush the entire mm.
> - */
Hm are we losing information here? I guess it's hard to know whewre to put
this though.
> -static inline void arch_flush_tlb_batched_pending(struct mm_struct *mm)
> -{
> - flush_tlb_mm(mm);
> -}
> -
> /*
> * To support TLB batched flush for multiple pages unmapping, we only send
> * the TLBI for each page in arch_tlbbatch_add_pending() and wait for the
> diff --git a/arch/riscv/include/asm/tlbflush.h b/arch/riscv/include/asm/tlbflush.h
> index 1a20dd746a49..eed0abc40514 100644
> --- a/arch/riscv/include/asm/tlbflush.h
> +++ b/arch/riscv/include/asm/tlbflush.h
> @@ -63,7 +63,6 @@ void flush_pud_tlb_range(struct vm_area_struct *vma, unsigned long start,
> bool arch_tlbbatch_should_defer(struct mm_struct *mm);
> void arch_tlbbatch_add_pending(struct arch_tlbflush_unmap_batch *batch,
> struct mm_struct *mm, unsigned long start, unsigned long end);
> -void arch_flush_tlb_batched_pending(struct mm_struct *mm);
> void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch);
>
> extern unsigned long tlb_flush_all_threshold;
> diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c
> index e737ba7949b1..8404530ec00f 100644
> --- a/arch/riscv/mm/tlbflush.c
> +++ b/arch/riscv/mm/tlbflush.c
> @@ -234,11 +234,6 @@ void arch_tlbbatch_add_pending(struct arch_tlbflush_unmap_batch *batch,
> mmu_notifier_arch_invalidate_secondary_tlbs(mm, start, end);
> }
>
> -void arch_flush_tlb_batched_pending(struct mm_struct *mm)
> -{
> - flush_tlb_mm(mm);
> -}
> -
> void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch)
> {
> __flush_tlb_range(NULL, &batch->cpumask,
> diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
> index e9b81876ebe4..00daedfefc1b 100644
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -356,11 +356,6 @@ static inline void arch_tlbbatch_add_pending(struct arch_tlbflush_unmap_batch *b
> mmu_notifier_arch_invalidate_secondary_tlbs(mm, 0, -1UL);
> }
>
> -static inline void arch_flush_tlb_batched_pending(struct mm_struct *mm)
> -{
> - flush_tlb_mm(mm);
> -}
> -
> extern void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch);
>
> static inline bool pte_flags_need_flush(unsigned long oldflags,
> diff --git a/mm/rmap.c b/mm/rmap.c
> index fb63d9256f09..fd160ddaa980 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -746,7 +746,7 @@ void flush_tlb_batched_pending(struct mm_struct *mm)
> int flushed = batch >> TLB_FLUSH_BATCH_FLUSHED_SHIFT;
>
> if (pending != flushed) {
> - arch_flush_tlb_batched_pending(mm);
> + flush_tlb_mm(mm);
I see that CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH is only implemented in
riscv (if !nommu), x86, arm64, and therefore we are only going to invoke
this for those arches which previously did the same anyway, so this is
safe.
Kinda wish we could avoid this ugly #ifdef #else #endif pattern here in
mm/rmap.c but probably necessary in this case.
> /*
> * If the new TLB flushing is pending during flushing, leave
> * mm->tlb_flush_batched as is, to avoid losing flushing.
> --
> 2.43.0
>
Powered by blists - more mailing lists