Re: [RFC PATCH 07/11] KVM: guest_memfd: Add ops-driven page revocation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm_gmem_invalidate_range() fails to invalidate GFNs that span multiple memslots, start in an unmapped hole, or exist in non-zero address spaces, leading to partial invalidation and potential Use-After-Free.
--

--- Patch [7]: [RFC PATCH 07/11] KVM: guest_memfd: Add ops-driven page revocation ---
commit 9715bf20df84962c5cc8863e7f4350b3bcb15273
Author: David Woodhouse <dwmw@amazon.co.uk>

KVM: guest_memfd: Add ops-driven page revocation

This commit adds kvm_gmem_invalidate_range() to allow a guest_memfd backing
to zap the guest secondary-MMU mapping for a gfn range. This provides an
implementation-to-KVM path to reclaim pages from a running guest.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 2b277468a12f0..6c13742de0680 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c

[ ... ]

> @@ -738,6 +738,45 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
>  	return __kvm_gmem_create(kvm, size, flags);
>  }
>  
> +/*
> + * Outbound (implementation -> KVM) revocation: an implementation zaps the
> + * guest's secondary MMU mapping for a gfn range so the next guest access
> + * re-faults through ops->get_pfn().  Mirrors __kvm_gmem_invalidate_start()
> + * for a single bound slot, bracketing the unmap with the mmu_invalidate
> + * window so a racing vCPU fault retries rather than installing a stale
> + * mapping.
> + */
> +void kvm_gmem_invalidate_range(struct kvm *kvm, gfn_t start, gfn_t end)
> +{
> +	struct kvm_memory_slot *slot;
> +	bool flush = false;
> +	int idx;
> +
> +	idx = srcu_read_lock(&kvm->srcu);
> +
> +	slot = gfn_to_memslot(kvm, start);
> +	if (slot) {
> +		struct kvm_gfn_range gfn_range = {
> +			.slot		= slot,
> +			.start		= start,
> +			.end		= min(end, slot->base_gfn + slot->npages),
> +			.may_block	= true,
> +			.attr_filter	= KVM_FILTER_SHARED | KVM_FILTER_PRIVATE,
> +		};

[Severity: High]
Does this code fully invalidate the requested range if it spans multiple
memslots or starts in an unmapped hole?

By looking up only the memslot for the start GFN and clamping the end to
slot->base_gfn + slot->npages, it seems any remaining GFNs in the requested
span will be skipped if the range crosses a memslot boundary.

Additionally, if the start GFN happens to fall in an unmapped hole,
gfn_to_memslot() returns NULL and the entire invalidation is bypassed, even
if there are valid mapped memslots further along in the range.

Finally, gfn_to_memslot() only checks Address Space 0. Could this leave
stale SPTEs in alternate address spaces (like SMM), allowing the guest to
access reclaimed host memory?

> +
> +		KVM_MMU_LOCK(kvm);
> +		kvm_mmu_invalidate_start(kvm);
> +		flush = kvm_mmu_unmap_gfn_range(kvm, &gfn_range);
> +		if (flush)
> +			kvm_flush_remote_tlbs(kvm);
> +		kvm_mmu_invalidate_end(kvm);
> +		KVM_MMU_UNLOCK(kvm);
> +	}
> +
> +	srcu_read_unlock(&kvm->srcu, idx);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/f09647f427565c1789a6979470045aab78661563.1784194290.git.dwmw@amazon.co.uk?part=7