From: Sean Christopherson seanjc@google.com
commit 93984f19e7bce4c18084a6ef3dacafb155b806ed upstream.
Protect gfn=>pfn cache refresh with a mutex to fully serialize refreshes. The refresh logic doesn't protect against
- concurrent unmaps, or refreshes with different GPAs (which may or may not happen in practice, for example if a cache is only used under vcpu->mutex; but it's allowed in the code)
- a false negative on the memslot generation. If the first refresh sees a stale memslot generation, it will refresh the hva and generation before moving on to the hva=>pfn translation. If it then drops gpc->lock, a different user of the cache can come along, acquire gpc->lock, see that the memslot generation is fresh, and skip the hva=>pfn update due to the userspace address also matching (because it too was updated).
The refresh path can already sleep during hva=>pfn resolution, so wrap the refresh with a mutex to ensure that any given refresh runs to completion before other callers can start their refresh.
Cc: stable@vger.kernel.org Cc: Lai Jiangshan jiangshanlai@gmail.com Signed-off-by: Sean Christopherson seanjc@google.com Message-Id: 20220429210025.3293691-7-seanjc@google.com Signed-off-by: Paolo Bonzini pbonzini@redhat.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- include/linux/kvm_types.h | 2 ++ virt/kvm/pfncache.c | 12 ++++++++++++ 2 files changed, 14 insertions(+)
--- a/include/linux/kvm_types.h +++ b/include/linux/kvm_types.h @@ -19,6 +19,7 @@ struct kvm_memslots; enum kvm_mr_change;
#include <linux/bits.h> +#include <linux/mutex.h> #include <linux/types.h> #include <linux/spinlock_types.h>
@@ -69,6 +70,7 @@ struct gfn_to_pfn_cache { struct kvm_vcpu *vcpu; struct list_head list; rwlock_t lock; + struct mutex refresh_lock; void *khva; kvm_pfn_t pfn; enum pfn_cache_usage usage; --- a/virt/kvm/pfncache.c +++ b/virt/kvm/pfncache.c @@ -157,6 +157,13 @@ int kvm_gfn_to_pfn_cache_refresh(struct if (page_offset + len > PAGE_SIZE) return -EINVAL;
+ /* + * If another task is refreshing the cache, wait for it to complete. + * There is no guarantee that concurrent refreshes will see the same + * gpa, memslots generation, etc..., so they must be fully serialized. + */ + mutex_lock(&gpc->refresh_lock); + write_lock_irq(&gpc->lock);
old_pfn = gpc->pfn; @@ -250,6 +257,8 @@ int kvm_gfn_to_pfn_cache_refresh(struct out: write_unlock_irq(&gpc->lock);
+ mutex_unlock(&gpc->refresh_lock); + gpc_release_pfn_and_khva(kvm, old_pfn, old_khva);
return ret; @@ -261,6 +270,7 @@ void kvm_gfn_to_pfn_cache_unmap(struct k void *old_khva; kvm_pfn_t old_pfn;
+ mutex_lock(&gpc->refresh_lock); write_lock_irq(&gpc->lock);
gpc->valid = false; @@ -276,6 +286,7 @@ void kvm_gfn_to_pfn_cache_unmap(struct k gpc->pfn = KVM_PFN_ERR_FAULT;
write_unlock_irq(&gpc->lock); + mutex_unlock(&gpc->refresh_lock);
gpc_release_pfn_and_khva(kvm, old_pfn, old_khva); } @@ -290,6 +301,7 @@ int kvm_gfn_to_pfn_cache_init(struct kvm
if (!gpc->active) { rwlock_init(&gpc->lock); + mutex_init(&gpc->refresh_lock);
gpc->khva = NULL; gpc->pfn = KVM_PFN_ERR_FAULT;