All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: reject VMA straddling the dirty ring on mmap
@ 2026-10-30 22:36 Rik van Riel
  2026-10-30 22:50 ` sashiko-bot
  2026-10-31 18:47 ` Paolo Bonzini
  0 siblings, 2 replies; 3+ messages in thread
From: Rik van Riel @ 2026-10-30 22:36 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Sean Christopherson, kvm, linux-kernel, kernel-team

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.

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.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 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;
 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: reject VMA straddling the dirty ring on mmap
  2026-10-30 22:36 [PATCH] KVM: reject VMA straddling the dirty ring on mmap Rik van Riel
@ 2026-10-30 22:50 ` sashiko-bot
  2026-10-31 18:47 ` Paolo Bonzini
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-30 22:50 UTC (permalink / raw)
  To: Rik van Riel; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: false-positive overlap in kvm_vcpu_mmap() when dirty ring is disabled
--

commit af0169eef51e967cdbe4222615548505445b40f5
Author: Rik van Riel <riel@surriel.com>

KVM: reject VMA straddling the dirty ring on mmap

This commit updates kvm_vcpu_mmap() to test for interval overlap against the
dirty-ring page range instead of only testing the VMA's first and last pages,
preventing MAP_PRIVATE or VM_EXEC mappings from spanning across the dirty
ring.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index cf4cb586f1407..97b944c77b347 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4178,10 +4178,16 @@ 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;

[Severity: Medium]
What happens in virt/kvm/kvm_main.c:kvm_vcpu_mmap() when the dirty ring is
not enabled on the VM (kvm->dirty_ring_size == 0)?

Previously, kvm_page_in_dirty_ring() checked:

virt/kvm/kvm_main.c:kvm_page_in_dirty_ring() {
#ifdef CONFIG_HAVE_KVM_DIRTY_RING
	return (pgoff >= KVM_DIRTY_LOG_PAGE_OFFSET) &&
	    (pgoff < KVM_DIRTY_LOG_PAGE_OFFSET +
	     kvm->dirty_ring_size / PAGE_SIZE);
#else
	return false;
#endif
}

which always returned false when kvm->dirty_ring_size was 0 or when
CONFIG_HAVE_KVM_DIRTY_RING was disabled.

With the new interval check, when kvm->dirty_ring_size is 0, ring_end equals
ring_start (KVM_DIRTY_LOG_PAGE_OFFSET), forming an empty interval
[ring_start, ring_start). However, the half-open overlap test:

	vma->vm_pgoff < ring_end && vma->vm_pgoff + pages > ring_start

assumes both intervals are non-empty. If userspace creates a MAP_PRIVATE or
VM_EXEC mapping on the vCPU fd that straddles KVM_DIRTY_LOG_PAGE_OFFSET
(vma->vm_pgoff < KVM_DIRTY_LOG_PAGE_OFFSET and
vma->vm_pgoff + pages > KVM_DIRTY_LOG_PAGE_OFFSET) on a VM with
kvm->dirty_ring_size == 0, won't kvm_vcpu_mmap() falsely reject the mapping
with -EINVAL?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030183604.3e8e373c@fangorn?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: reject VMA straddling the dirty ring on mmap
  2026-10-30 22:36 [PATCH] KVM: reject VMA straddling the dirty ring on mmap Rik van Riel
  2026-10-30 22:50 ` sashiko-bot
@ 2026-10-31 18:47 ` Paolo Bonzini
  1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2026-10-31 18:47 UTC (permalink / raw)
  To: Rik van Riel
  Cc: Sean Christopherson, kvm, Kernel Mailing List, Linux, kernel-team

Il ven 30 ott 2026, 23:36 Rik van Riel <riel@surriel.com> 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@surriel.com>
> ---
>  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;
>
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-31 18:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-30 22:36 [PATCH] KVM: reject VMA straddling the dirty ring on mmap Rik van Riel
2026-10-30 22:50 ` sashiko-bot
2026-10-31 18:47 ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.