Re: [PATCH] KVM: reject VMA straddling the dirty ring on mmap

[Paolo Bonzini]

Il ven 30 ott 2026, 23:36 Rik van Riel <riel@xxxxxxxxxxx> ha scritto:
>
> kvm_vcpu_mmap only tests the VMA's first and last page against the
> dirty-ring interval, in order to prevent a MAP_PRIVATE VMA from
> getting mapped into the dirty ring, breaking dirty tracking.
>
> A VMA that starts before the ring and ends after it has neither
> endpoint inside, so the check passes even though the
> mapping covers the ring.
>
> Getting this wrong is observable from host userspace: the VMM
> (QEMU) mmaps the vCPU fd to reach the dirty ring, and a MAP_PRIVATE
> mapping that spans the ring succeeds even though it covers the ring.
> Faulting one instantiates a private CoW copy instead of the shared
> ring page. The guest's dirty-ring writes then land in a page the
> VMM never sees, silently breaking dirty-page tracking.

This is deep in "if it hurts don't do it" territory. Unless I am
missing something huge, it's at best a decent code cleanup. "Silently
breaking" is overselling by a lot...

> Fix by testing interval overlap instead of endpoint membership.
>
> Tested by creating a VM with a 16-page dirty ring. Mmap the
> VCPU fd covering pgoff 0...199 MAP_PRIVATE, then fault a
> page inside the ring. Succeeds before this patch, -EINVAL after.

I probably would have applied it, but the commit message and
especially the comment in AI English made me change my mind quickly.

You've been working on Linux for 30 years, I would have expected a
higher standard; and if this is the kind of commit in Meta's
8000-patch stash, I'm not sure I want to look at the rest...

Paolo

> Signed-off-by: Rik van Riel <riel@xxxxxxxxxxx>
> ---
>  virt/kvm/kvm_main.c | 10 +++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4175,9 +4175,15 @@ static const struct vm_operations_struct kvm_vcpu_vm_ops = {
>  static int kvm_vcpu_mmap(struct file *file, struct vm_area_struct *vma)
>  {
>         struct kvm_vcpu *vcpu = file->private_data;
> +       struct kvm *kvm = vcpu->kvm;
>         unsigned long pages = vma_pages(vma);
> +       unsigned long ring_start = KVM_DIRTY_LOG_PAGE_OFFSET;
> +       unsigned long ring_end = ring_start + kvm->dirty_ring_size / PAGE_SIZE;
>
> -       if ((kvm_page_in_dirty_ring(vcpu->kvm, vma->vm_pgoff) ||
> -            kvm_page_in_dirty_ring(vcpu->kvm, vma->vm_pgoff + pages - 1)) &&
> +       /*
> +        * An overlapping VMA is rejected when private or executable: a CoW
> +        * fault hides guest writes from the VMM.
> +        */
> +       if (vma->vm_pgoff < ring_end && vma->vm_pgoff + pages > ring_start &&
>             ((vma->vm_flags & VM_EXEC) || !(vma->vm_flags & VM_SHARED)))
>                 return -EINVAL;
>
>