Re: [PATCH v3 6/8] KVM: Fully serialize gfn=>pfn cache refresh via mutex

[Paolo Bonzini]

On 5/20/22 17:53, Sean Christopherson wrote:
> > Does kvm_gfn_to_pfn_cache_unmap also need to take the mutex, to avoid the
> > WARN_ON(gpc->valid)?
> I don't know What WARN_ON() you're referring to, but there is a double-free bug
> if unmap() runs during an invalidation.  That can be solved without having to
> take the mutex though, just reset valid/pfn/khva before the retry.

I was thinking of this one:

	/*
	 * Other tasks must wait for _this_ refresh to complete before
	 * attempting to refresh.
	 */
	WARN_ON_ONCE(gpc->valid);

but unmap sets gpc->valid to false, not true. ಠ_ಠ

Still, as you point out unmap() and refresh() can easily clash.  In 
practice they probably exclude each other by different means (e.g. 
running in a single vCPU thread), but then in practice neither is a fast 
path.  It seems easier to just make them thread-safe the easy way now 
that there is a mutex.

> When searching to see how unmap() was used in the original series (there's no
> other user besides destroy...), I stumbled across this likely-related syzbot bug
> that unfortunately didn't Cc KVM:-(

To give an example, VMCLEAR would do an unmap() if the VMCS12 was mapped 
with a gfn-to-pfn cache.

Paolo