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 <[email protected]> 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.d...@amazon.co.uk?part=7
