Re: [PATCH 5.10.y] KVM: arm64: Retry fault if vma_lookup() results become invalid

Bjoern Doebel <[email protected]>
Newsgroups dev.linux.lists.sashiko-reviews,dev.linux.lists.kvmarm
Message-ID <aogeHAy/[email protected]>
On Fri, Aug 21, 2026 at 09:14:39AM +0000, [email protected] wrote:
> > -	}
> > -
> >  	mmu_seq = vcpu->kvm->mmu_notifier_seq;
> > -	/*
> > -	 * Ensure the read of mmu_notifier_seq happens before we call
> > -	 * gfn_to_pfn_prot (which calls get_user_pages), so that we don't risk
> > -	 * the page we just got a reference to gets unmapped before we have a
> > -	 * chance to grab the mmu_lock, which ensure that if the page gets
> > -	 * unmapped afterwards, the call to kvm_unmap_hva will take it away
> > -	 * from us again properly. This smp_rmb() interacts with the smp_wmb()
> > -	 * in kvm_mmu_notifier_invalidate_<page|range_end>.
> > -	 */
> > -	smp_rmb();
> > +	/*
> > +	 * Read mmu_notifier_seq so that KVM can detect if the results of
> > +	 * find_vma_intersection() or gfn_to_pfn_prot() become stale prior to
> > +	 * acquiring kvm->mmu_lock.
> > +	 *
> > +	 * Rely on mmap_read_unlock() for an implicit smp_rmb(), which pairs
> > +	 * with the smp_wmb() in kvm_mmu_notifier_invalidate_range_end().
> > +	 */
> > +	mmap_read_unlock(current->mm);
> >  
> >  	pfn = gfn_to_pfn_prot(kvm, gfn, write_fault, &writable);
> 
> [Severity: Critical]
> Does removing the explicit smp_rmb() here in user_mem_abort() break the MMU
> notifier retry protocol?
> 
> The comment states that we can rely on mmap_read_unlock() for an implicit
> smp_rmb(). However, mmap_read_unlock() via up_read() typically provides only
> RELEASE semantics.

Note that this removal of smp_rmb() is not new in this backport but
verbatim taken from the mainline patch. It is also contained in all
previous backports of the patch.

I'm not an expert on the memory semantics here and open to suggestions.
But _if_ we consider this a problem, we would have to go back and review
mainline and the other backports as well.

Bjoern
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.