Skip to content

Commit 5c9ca4e

Browse files
committed
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: [email protected] Closes: https://lore.kernel.org/all/[email protected] Fixes: 721f5b0 ("KVM: pfncache: allow a cache to be activated with a fixed (userspace) HVA") Cc: David Woodhouse <[email protected]> Cc: Paul Durrant <[email protected]> Reviewed-by: Paul Durrant <[email protected]> Reviewed-by: David Woodhouse <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Sean Christopherson <[email protected]>
1 parent 18f06e9 commit 5c9ca4e

File tree

1 file changed

+9
-7
lines changed

1 file changed

+9
-7
lines changed

virt/kvm/pfncache.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,7 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
245245
return -EFAULT;
246246
}
247247

248-
static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva,
249-
unsigned long len)
248+
static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva)
250249
{
251250
unsigned long page_offset;
252251
bool unmap_old = false;
@@ -260,9 +259,6 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
260259
if (WARN_ON_ONCE(kvm_is_error_gpa(gpa) == kvm_is_error_hva(uhva)))
261260
return -EINVAL;
262261

263-
if (!kvm_gpc_is_valid_len(gpa, uhva, len))
264-
return -EINVAL;
265-
266262
lockdep_assert_held(&gpc->refresh_lock);
267263

268264
write_lock_irq(&gpc->lock);
@@ -365,14 +361,17 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len)
365361

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

364+
if (!kvm_gpc_is_valid_len(gpc->gpa, gpc->uhva, len))
365+
return -EINVAL;
366+
368367
/*
369368
* If the GPA is valid then ignore the HVA, as a cache can be GPA-based
370369
* or HVA-based, not both. For GPA-based caches, the HVA will be
371370
* recomputed during refresh if necessary.
372371
*/
373372
uhva = kvm_is_error_gpa(gpc->gpa) ? gpc->uhva : KVM_HVA_ERR_BAD;
374373

375-
return __kvm_gpc_refresh(gpc, gpc->gpa, uhva, len);
374+
return __kvm_gpc_refresh(gpc, gpc->gpa, uhva);
376375
}
377376

378377
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
392391
{
393392
struct kvm *kvm = gpc->kvm;
394393

394+
if (!kvm_gpc_is_valid_len(gpa, uhva, len))
395+
return -EINVAL;
396+
395397
guard(mutex)(&gpc->refresh_lock);
396398

397399
if (!gpc->active) {
@@ -411,7 +413,7 @@ static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned
411413
gpc->active = true;
412414
write_unlock_irq(&gpc->lock);
413415
}
414-
return __kvm_gpc_refresh(gpc, gpa, uhva, len);
416+
return __kvm_gpc_refresh(gpc, gpa, uhva);
415417
}
416418

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

0 commit comments

Comments
 (0)