linux-kernel - Re: [PATCH v2] mmu_gather: move tlb flush for VM_PFNMAP/VM_MIXEDMAP vmas into free

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: <20250124082250.GA13226@noisy.programming.kicks-ass.net>
Date: Fri, 24 Jan 2025 09:22:50 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Roman Gushchin <roman.gushchin@...ux.dev>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jann Horn <jannh@...gle.com>, Will Deacon <will@...nel.org>,
	"Aneesh Kumar K.V" <aneesh.kumar@...nel.org>,
	Nick Piggin <npiggin@...il.com>, Hugh Dickins <hughd@...gle.com>,
	linux-arch@...r.kernel.org
Subject: Re: [PATCH v2] mmu_gather: move tlb flush for VM_PFNMAP/VM_MIXEDMAP
 vmas into free_pgtables()

On Thu, Jan 23, 2025 at 11:12:33PM +0000, Roman Gushchin wrote:

> > +static inline void tlb_free_vma(struct mmu_gather *tlb, struct vm_area_struct *vma)
> >  {
> >  	if (tlb->fullmm)
> >  		return;
> >  
> >  	/*
> >  	 * VM_PFNMAP is more fragile because the core mm will not track the
> > +	 * page mapcount -- there might not be page-frames for these PFNs
> > +	 * after all.
> > +	 *
> > +	 * Specifically() there is a race between munmap() and
> > +	 * unmap_mapping_range(), where munmap() will unlink the VMA, such
> > +	 * that unmap_mapping_range() will no longer observe the VMA and
> > +	 * no-op, without observing the TLBI, returning prematurely.
> > +	 *
> > +	 * So if we're about to unlink such a VMA, and we have pending
> > +	 * TLBI for such a vma, flush things now.
> >  	 */
> > +	if ((vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)) && tlb->vma_pfn)
> >  		tlb_flush_mmu_tlbonly(tlb);
> 
> Why do we need to re-check vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP) here?

No need, but an opportunity.

> In free_pgtables() we're iterating over multiple vma's. What if the first has
> no VM_PFNMAP set, but some other do? Idk if it's even possible, but it's not
> obvious that it's not possible either.

If we only need to flush PFN entries before unlinking PFN VMAs, then:

 - if there are no PFNs pending (vma_pfn), we don't need to flush;
 - if no PFN vma is being freed (vm_flags), we don't need to flush.

Notably, if an earlier flush has already issued the TLBI, there is no
need to issue one again, but also, if we end up not actually freeing the
PFN vma, we also don't care.