Commit 5c9ca4ed authored by Sean Christopherson's avatar Sean Christopherson
Browse files

KVM: Check validity of offset+length of gfn_to_pfn_cache prior to activation 



When activating a gfn_to_pfn_cache, verify that the offset+length is sane
and usable before marking the cache active.  Letting __kvm_gpc_refresh()
detect the problem results in a cache being marked active without setting
the GPA (or any other fields), which in turn results in KVM trying to
refresh a cache with INVALID_GPA.

Attempting to refresh a cache with INVALID_GPA isn't functionally
problematic, but it runs afoul of the sanity check that exactly one of
GPA or userspace HVA is valid, i.e. that a cache is either GPA-based or
HVA-based.

Reported-by: default avatar <syzbot+106a4f72b0474e1d1b33@syzkaller.appspotmail.com>
Closes: https://lore.kernel.org/all/0000000000005fa5cc0613f1cebd@google.com


Fixes: 721f5b0d ("KVM: pfncache: allow a cache to be activated with a fixed (userspace) HVA")
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Paul Durrant <paul@xen.org>
Reviewed-by: default avatarPaul Durrant <paul@xen.org>
Reviewed-by: default avatarDavid Woodhouse <dwmw@amazon.co.uk>
Link: https://lore.kernel.org/r/20240320001542.3203871-3-seanjc@google.com


Signed-off-by: default avatarSean Christopherson <seanjc@google.com>
parent 18f06e97

virt/kvm/pfncache.c

+9 −7
Original line number Diff line number Diff line
@@ -245,8 +245,7 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc) 
	return -EFAULT;

}



static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva,

			     unsigned long len)

static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva)

{

	unsigned long page_offset;

	bool unmap_old = false;
@@ -260,9 +259,6 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l 
	if (WARN_ON_ONCE(kvm_is_error_gpa(gpa) == kvm_is_error_hva(uhva)))

		return -EINVAL;



	if (!kvm_gpc_is_valid_len(gpa, uhva, len))

		return -EINVAL;



	lockdep_assert_held(&gpc->refresh_lock);



	write_lock_irq(&gpc->lock);
@@ -365,6 +361,9 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len) 


	guard(mutex)(&gpc->refresh_lock);



	if (!kvm_gpc_is_valid_len(gpc->gpa, gpc->uhva, len))

		return -EINVAL;



	/*

	 * If the GPA is valid then ignore the HVA, as a cache can be GPA-based

	 * or HVA-based, not both.  For GPA-based caches, the HVA will be
@@ -372,7 +371,7 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len) 
	 */

	uhva = kvm_is_error_gpa(gpc->gpa) ? gpc->uhva : KVM_HVA_ERR_BAD;



	return __kvm_gpc_refresh(gpc, gpc->gpa, uhva, len);

	return __kvm_gpc_refresh(gpc, gpc->gpa, uhva);

}



void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
@@ -392,6 +391,9 @@ static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned 
{

	struct kvm *kvm = gpc->kvm;



	if (!kvm_gpc_is_valid_len(gpa, uhva, len))

		return -EINVAL;



	guard(mutex)(&gpc->refresh_lock);



	if (!gpc->active) {
@@ -411,7 +413,7 @@ static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned 
		gpc->active = true;

		write_unlock_irq(&gpc->lock);

	}

	return __kvm_gpc_refresh(gpc, gpa, uhva, len);

	return __kvm_gpc_refresh(gpc, gpa, uhva);

}



int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len)