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