Re: [PATCH v6 7/8] KVM: Enable and expose KVM_MEM_PRIVATE

From: Michael Roth
Date: Thu Jun 23 2022 - 19:00:16 EST


On Thu, May 19, 2022 at 11:37:12PM +0800, Chao Peng wrote:
> Register private memslot to fd-based memory backing store and handle the
> memfile notifiers to zap the existing mappings.
>
> Currently the register is happened at memslot creating time and the
> initial support does not include page migration/swap.
>
> KVM_MEM_PRIVATE is not exposed by default, architecture code can turn
> on it by implementing kvm_arch_private_mem_supported().
>
> A 'kvm' reference is added in memslot structure since in
> memfile_notifier callbacks we can only obtain a memslot reference while
> kvm is need to do the zapping. The zapping itself reuses code from
> existing mmu notifier handling.
>
> Co-developed-by: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>
> Signed-off-by: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>
> Signed-off-by: Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>
> ---
> include/linux/kvm_host.h | 10 ++-
> virt/kvm/kvm_main.c | 132 ++++++++++++++++++++++++++++++++++++---
> 2 files changed, 131 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index b0a7910505ed..00efb4b96bc7 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -246,7 +246,7 @@ bool kvm_setup_async_pf(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);
> #endif
>
> -#ifdef KVM_ARCH_WANT_MMU_NOTIFIER
> +#if defined(KVM_ARCH_WANT_MMU_NOTIFIER) || defined(CONFIG_MEMFILE_NOTIFIER)
> struct kvm_gfn_range {
> struct kvm_memory_slot *slot;
> gfn_t start;
> @@ -577,6 +577,7 @@ struct kvm_memory_slot {
> struct file *private_file;
> loff_t private_offset;
> struct memfile_notifier notifier;
> + struct kvm *kvm;
> };
>
> static inline bool kvm_slot_is_private(const struct kvm_memory_slot *slot)
> @@ -769,9 +770,13 @@ struct kvm {
> struct hlist_head irq_ack_notifier_list;
> #endif
>
> +#if (defined(CONFIG_MMU_NOTIFIER) && defined(KVM_ARCH_WANT_MMU_NOTIFIER)) ||\
> + defined(CONFIG_MEMFILE_NOTIFIER)
> + unsigned long mmu_notifier_seq;
> +#endif
> +
> #if defined(CONFIG_MMU_NOTIFIER) && defined(KVM_ARCH_WANT_MMU_NOTIFIER)
> struct mmu_notifier mmu_notifier;
> - unsigned long mmu_notifier_seq;
> long mmu_notifier_count;
> unsigned long mmu_notifier_range_start;
> unsigned long mmu_notifier_range_end;
> @@ -1438,6 +1443,7 @@ bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu);
> int kvm_arch_post_init_vm(struct kvm *kvm);
> void kvm_arch_pre_destroy_vm(struct kvm *kvm);
> int kvm_arch_create_vm_debugfs(struct kvm *kvm);
> +bool kvm_arch_private_mem_supported(struct kvm *kvm);
>
> #ifndef __KVM_HAVE_ARCH_VM_ALLOC
> /*
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index db9d39a2d3a6..f93ac7cdfb53 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -843,6 +843,73 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
>
> #endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
>
> +#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM
> +static void kvm_private_mem_notifier_handler(struct memfile_notifier *notifier,
> + pgoff_t start, pgoff_t end)
> +{
> + int idx;
> + struct kvm_memory_slot *slot = container_of(notifier,
> + struct kvm_memory_slot,
> + notifier);
> + struct kvm_gfn_range gfn_range = {
> + .slot = slot,
> + .start = start - (slot->private_offset >> PAGE_SHIFT),
> + .end = end - (slot->private_offset >> PAGE_SHIFT),

This code assumes that 'end' is greater than slot->private_offset, but
even if slot->private_offset is non-zero, nothing stops userspace from
allocating pages in the range of 0 through slot->private_offset, which
will still end up triggering this notifier. In that case gfn_range.end
will end up going negative, and the below code will limit that to
slot->npages and do a populate/invalidate for the entire range.

Not sure if this covers all the cases, but this fixes the issue for me:

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 903ffdb5f01c..4c744d8f7527 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -872,6 +872,19 @@ static void kvm_private_mem_notifier_handler(struct memfile_notifier *notifier,
.may_block = true,
};

struct kvm *kvm = slot->kvm;
+
+ if (slot->private_offset > end)
+ return;
+