The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] kvm: apply VM_READ/VM_WRITE checks to all VMA types
@ 2026-07-31 17:58 Paolo Bonzini
  2026-07-31 18:46 ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2026-07-31 17:58 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Sean Christopherson

The VM_READ and VM_WRITE flags are checked only at the very end of
hva_to_pfn().  For both the hva_to_pfn_remapped() case and for regular
mappings, this adds unnecessary cases and inconsistent error behavior.

For hva_to_pfn_remapped(), the code is relying on fixup_user_fault() to
detect this situation.  This is fragile because hva_to_pfn_remapped()
returns different error codes for a !VM_WRITE VMA depending on whether
the PTE happens to be mapped:

* if the PTE is present, follow_pfnmap_start() sets args.writable to
  false and KVM_PFN_ERR_RO_FAULT is returned;

* if no PTE is present, fixup_user_fault(FAULT_FLAG_WRITE) returns
  -EFAULT after checking vma_permits_fault(), and hva_to_pfn() ends
  up returning KVM_PFN_ERR_FAULT.

With this patch KVM_PFN_ERR_RO_FAULT is returned uniformly.  Likewise,
a PROT_NONE pfnmap VMA would be mapped into the guest if the PTE was
pte_present()[1] when the guest attempted to read it; with the patch
instead KVM uniformly returns KVM_PFN_ERR_FAULT.  Doing the check early
avoids these special cases and also sidesteps the issue pointed out at
https://sashiko.dev/#/patchset/20260731160514.1101989-1-pbonzini%40redhat.com.

For regular mappings a PROT_READ VMA, if placed in a writable memslot,
would return KVM_PFN_ERR_FAULT instead of KVM_PFN_ERR_RO_FAULT when
the guest writes to it.  This would cause a -EFAULT exit to userspace,
instead of triggering emulation as the VM_IO|VM_PFNMAP arm would do;
however it should be considered part of the KVM API because mmu_stress_test
relies on it.

Still, even with this snag about the returned pfn error code, pull the
vm_flags checks in front so that they are done for all VMAs and the
above inconsistency goes away for the VM_IO|VM_PFNMAP case.

[1] on x86, for example, such a page would have _PAGE_PRESENT clear
  but _PAGE_PROTNONE set

Cc: Sean Christopherson <seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 virt/kvm/kvm_main.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 45e784462ec6..eaf8ee0725d4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2925,17 +2925,6 @@ static int hva_to_pfn_slow(struct kvm_follow_pfn *kfp, kvm_pfn_t *pfn)
 	return npages;
 }
 
-static bool vma_is_valid(struct vm_area_struct *vma, bool write_fault)
-{
-	if (unlikely(!(vma->vm_flags & VM_READ)))
-		return false;
-
-	if (write_fault && (unlikely(!(vma->vm_flags & VM_WRITE))))
-		return false;
-
-	return true;
-}
-
 static int hva_to_pfn_remapped(struct vm_area_struct *vma,
 			       struct kvm_follow_pfn *kfp, kvm_pfn_t *p_pfn)
 {
@@ -3008,20 +2997,29 @@ kvm_pfn_t hva_to_pfn(struct kvm_follow_pfn *kfp)
 retry:
 	vma = vma_lookup(current->mm, kfp->hva);
 
-	if (vma == NULL)
+	/*
+	 * GUP failed.  It could be an inaccessible mapping, a pfnmap one,
+	 * or the page might be absent.
+	 */
+
+	if (vma == NULL || unlikely(!(vma->vm_flags & VM_READ))) {
 		pfn = KVM_PFN_ERR_FAULT;
-	else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
+	} else if ((kfp->flags & FOLL_WRITE) && unlikely(!(vma->vm_flags & VM_WRITE))) {
+		/*
+		 * Exit to userspace for PROT_READ mappings in a writable
+		 * memslot, as this is part of the API.
+		 */
+		pfn = vma->vm_flags & (VM_IO | VM_PFNMAP)
+			? KVM_PFN_ERR_RO_FAULT : KVM_PFN_ERR_FAULT;
+	} else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
 		r = hva_to_pfn_remapped(vma, kfp, &pfn);
 		if (r == -EAGAIN)
 			goto retry;
 		if (r < 0)
 			pfn = KVM_PFN_ERR_FAULT;
 	} else {
-		if ((kfp->flags & FOLL_NOWAIT) &&
-		    vma_is_valid(vma, kfp->flags & FOLL_WRITE))
-			pfn = KVM_PFN_ERR_NEEDS_IO;
-		else
-			pfn = KVM_PFN_ERR_FAULT;
+		pfn = kfp->flags & FOLL_NOWAIT
+			? KVM_PFN_ERR_NEEDS_IO : KVM_PFN_ERR_FAULT;
 	}
 	mmap_read_unlock(current->mm);
 	return pfn;
-- 
2.55.0


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

* Re: [PATCH] kvm: apply VM_READ/VM_WRITE checks to all VMA types
  2026-07-31 17:58 [PATCH] kvm: apply VM_READ/VM_WRITE checks to all VMA types Paolo Bonzini
@ 2026-07-31 18:46 ` Sean Christopherson
  2026-08-03  5:54   ` Paolo Bonzini
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2026-07-31 18:46 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

On Fri, Jul 31, 2026, Paolo Bonzini wrote:
> The VM_READ and VM_WRITE flags are checked only at the very end of
> hva_to_pfn().  For both the hva_to_pfn_remapped() case and for regular
> mappings, this adds unnecessary cases and inconsistent error behavior.
> 
> For hva_to_pfn_remapped(), the code is relying on fixup_user_fault() to
> detect this situation.  This is fragile because hva_to_pfn_remapped()
> returns different error codes for a !VM_WRITE VMA depending on whether
> the PTE happens to be mapped:
> 
> * if the PTE is present, follow_pfnmap_start() sets args.writable to
>   false and KVM_PFN_ERR_RO_FAULT is returned;
> 
> * if no PTE is present, fixup_user_fault(FAULT_FLAG_WRITE) returns
>   -EFAULT after checking vma_permits_fault(), and hva_to_pfn() ends
>   up returning KVM_PFN_ERR_FAULT.
> 
> With this patch KVM_PFN_ERR_RO_FAULT is returned uniformly.

IMO, returning KVM_PFN_ERR_RO_FAULT on a read-only VMA is wrong.  AFAICT, that
behavior for VM_{IO,PFNMAP} was added by commit bd2fae8da794 ("KVM: do not assume
PTE is writable after follow_pfn").  Given that that's the only case where KVM
returns KVM_PFN_ERR_RO_FAULT, I would much prefer to fix that wart and cross our
fingers nothing has come to rely on the behavior in the last ~5 years.

> Likewise, a PROT_NONE pfnmap VMA would be mapped into the guest if the PTE
> was pte_present()[1] when the guest attempted to read it; with the patch
> instead KVM uniformly returns KVM_PFN_ERR_FAULT.  Doing the check early
> avoids these special cases and also sidesteps the issue pointed out at
> https://sashiko.dev/#/patchset/20260731160514.1101989-1-pbonzini%40redhat.com.
> 
> For regular mappings a PROT_READ VMA, if placed in a writable memslot,
> would return KVM_PFN_ERR_FAULT instead of KVM_PFN_ERR_RO_FAULT when
> the guest writes to it.  This would cause a -EFAULT exit to userspace,
> instead of triggering emulation as the VM_IO|VM_PFNMAP arm would do;

No, arm64 is checking the memslot, not the VMA.

	hva = gfn_to_hva_memslot_prot(memslot, gfn, &writable);
	write_fault = kvm_is_write_fault(vcpu);
	if (kvm_is_error_hva(hva) || (write_fault && !writable)) {

Or are you talking about different code?

> however it should be considered part of the KVM API because mmu_stress_test
> relies on it.

...

>  	} else {
> -		if ((kfp->flags & FOLL_NOWAIT) &&
> -		    vma_is_valid(vma, kfp->flags & FOLL_WRITE))
> -			pfn = KVM_PFN_ERR_NEEDS_IO;
> -		else
> -			pfn = KVM_PFN_ERR_FAULT;
> +		pfn = kfp->flags & FOLL_NOWAIT
> +			? KVM_PFN_ERR_NEEDS_IO : KVM_PFN_ERR_FAULT;

I find this style much easier to read:

		pfn = kfp->flags & FOLL_NOWAIT ? KVM_PFN_ERR_NEEDS_IO :
						 KVM_PFN_ERR_FAULT;

If we "fix" the read-only behavior, isn't it this?

	/*
	 * GUP failed.  It could be an inaccessible mapping, a pfnmap one,
	 * or the page might be absent.
	 */
	if (vma == NULL || unlikely(!(vma->vm_flags & VM_READ)) ||
	    ((kfp->flags & FOLL_WRITE) && unlikely(!(vma->vm_flags & VM_WRITE)))) {
		pfn = KVM_PFN_ERR_FAULT;
	} else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
		r = hva_to_pfn_remapped(vma, kfp, &pfn);
		if (r == -EAGAIN)
			goto retry;
		if (r < 0)
			pfn = KVM_PFN_ERR_FAULT;
	} else {
		pfn = kfp->flags & FOLL_NOWAIT ? KVM_PFN_ERR_NEEDS_IO :
						 KVM_PFN_ERR_FAULT;
	}


>  	}
>  	mmap_read_unlock(current->mm);
>  	return pfn;
> -- 
> 2.55.0
> 

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

* Re: [PATCH] kvm: apply VM_READ/VM_WRITE checks to all VMA types
  2026-07-31 18:46 ` Sean Christopherson
@ 2026-08-03  5:54   ` Paolo Bonzini
  2026-08-03 13:40     ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2026-08-03  5:54 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-kernel, kvm

On Fri, Jul 31, 2026 at 8:46 PM Sean Christopherson <seanjc@google.com> wrote:
> > * if the PTE is present, follow_pfnmap_start() sets args.writable to
> >   false and KVM_PFN_ERR_RO_FAULT is returned;
> >
> > * if no PTE is present, fixup_user_fault(FAULT_FLAG_WRITE) returns
> >   -EFAULT after checking vma_permits_fault(), and hva_to_pfn() ends
> >   up returning KVM_PFN_ERR_FAULT.
> >
> > With this patch KVM_PFN_ERR_RO_FAULT is returned uniformly.
>
> IMO, returning KVM_PFN_ERR_RO_FAULT on a read-only VMA is wrong.  AFAICT, that
> behavior for VM_{IO,PFNMAP} was added by commit bd2fae8da794 ("KVM: do not assume
> PTE is writable after follow_pfn").  Given that that's the only case where KVM
> returns KVM_PFN_ERR_RO_FAULT, I would much prefer to fix that wart and cross our
> fingers nothing has come to rely on the behavior in the last ~5 years.

We can try, but I'd rather not do that in stable releases (while this
patch would be applied there, as a first step towards fixing the DRM
issue that Sergio reported - it avoids the sashiko issue reported for
https://lore.kernel.org/r/20260731160514.1101989-1-pbonzini%40redhat.com/).

> > For regular mappings a PROT_READ VMA, if placed in a writable memslot,
> > would return KVM_PFN_ERR_FAULT instead of KVM_PFN_ERR_RO_FAULT when
> > the guest writes to it.  This would cause a -EFAULT exit to userspace,
> > instead of triggering emulation as the VM_IO|VM_PFNMAP arm would do;
>
> No, arm64 is checking the memslot, not the VMA.
>
>         hva = gfn_to_hva_memslot_prot(memslot, gfn, &writable);
>         write_fault = kvm_is_write_fault(vcpu);
>         if (kvm_is_error_hva(hva) || (write_fault && !writable)) {
>
> Or are you talking about different code?

I am talking about the "arm" of the if/else if/else. :)

>
>         /*
>          * GUP failed.  It could be an inaccessible mapping, a pfnmap one,
>          * or the page might be absent.
>          */
>         if (vma == NULL || unlikely(!(vma->vm_flags & VM_READ)) ||
>             ((kfp->flags & FOLL_WRITE) && unlikely(!(vma->vm_flags & VM_WRITE)))) {
>                 pfn = KVM_PFN_ERR_FAULT;
>         } else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
>                 r = hva_to_pfn_remapped(vma, kfp, &pfn);
>                 if (r == -EAGAIN)
>                         goto retry;
>                 if (r < 0)
>                         pfn = KVM_PFN_ERR_FAULT;
>         } else {
>                 pfn = kfp->flags & FOLL_NOWAIT ? KVM_PFN_ERR_NEEDS_IO :
>                                                  KVM_PFN_ERR_FAULT;
>         }

Yes, but I'd do that only in 7.3.

Paolo


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

* Re: [PATCH] kvm: apply VM_READ/VM_WRITE checks to all VMA types
  2026-08-03  5:54   ` Paolo Bonzini
@ 2026-08-03 13:40     ` Sean Christopherson
  2026-08-03 17:17       ` Paolo Bonzini
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2026-08-03 13:40 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

On Mon, Aug 03, 2026, Paolo Bonzini wrote:
> On Fri, Jul 31, 2026 at 8:46 PM Sean Christopherson <seanjc@google.com> wrote:
> > > * if the PTE is present, follow_pfnmap_start() sets args.writable to
> > >   false and KVM_PFN_ERR_RO_FAULT is returned;
> > >
> > > * if no PTE is present, fixup_user_fault(FAULT_FLAG_WRITE) returns
> > >   -EFAULT after checking vma_permits_fault(), and hva_to_pfn() ends
> > >   up returning KVM_PFN_ERR_FAULT.
> > >
> > > With this patch KVM_PFN_ERR_RO_FAULT is returned uniformly.
> >
> > IMO, returning KVM_PFN_ERR_RO_FAULT on a read-only VMA is wrong.  AFAICT, that
> > behavior for VM_{IO,PFNMAP} was added by commit bd2fae8da794 ("KVM: do not assume
> > PTE is writable after follow_pfn").  Given that that's the only case where KVM
> > returns KVM_PFN_ERR_RO_FAULT, I would much prefer to fix that wart and cross our
> > fingers nothing has come to rely on the behavior in the last ~5 years.
> 
> We can try, but I'd rather not do that in stable releases (while this
> patch would be applied there, as a first step towards fixing the DRM
> issue that Sergio reported 

I don't see how this would help with fixup_user_fault() not actually fixing a
fault.  Neither returning -EFAULT nor emulating is correct KVM behavior.

> - it avoids the sashiko issue reported for
> https://lore.kernel.org/r/20260731160514.1101989-1-pbonzini%40redhat.com/).

But the issue Sashiko reported is just saying that KVM sometimes does what I'm
saying KVM should do all the time: return -EFAULT.  Or did I misunderstand that
one too? :-)

> > > For regular mappings a PROT_READ VMA, if placed in a writable memslot,
> > > would return KVM_PFN_ERR_FAULT instead of KVM_PFN_ERR_RO_FAULT when
> > > the guest writes to it.  This would cause a -EFAULT exit to userspace,
> > > instead of triggering emulation as the VM_IO|VM_PFNMAP arm would do;
> >
> > No, arm64 is checking the memslot, not the VMA.
> >
> >         hva = gfn_to_hva_memslot_prot(memslot, gfn, &writable);
> >         write_fault = kvm_is_write_fault(vcpu);
> >         if (kvm_is_error_hva(hva) || (write_fault && !writable)) {
> >
> > Or are you talking about different code?
> 
> I am talking about the "arm" of the if/else if/else. :)

LOL, overthought that one a bit.

> >
> >         /*
> >          * GUP failed.  It could be an inaccessible mapping, a pfnmap one,
> >          * or the page might be absent.
> >          */
> >         if (vma == NULL || unlikely(!(vma->vm_flags & VM_READ)) ||
> >             ((kfp->flags & FOLL_WRITE) && unlikely(!(vma->vm_flags & VM_WRITE)))) {
> >                 pfn = KVM_PFN_ERR_FAULT;
> >         } else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
> >                 r = hva_to_pfn_remapped(vma, kfp, &pfn);
> >                 if (r == -EAGAIN)
> >                         goto retry;
> >                 if (r < 0)
> >                         pfn = KVM_PFN_ERR_FAULT;
> >         } else {
> >                 pfn = kfp->flags & FOLL_NOWAIT ? KVM_PFN_ERR_NEEDS_IO :
> >                                                  KVM_PFN_ERR_FAULT;
> >         }
> 
> Yes, but I'd do that only in 7.3.

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

* Re: [PATCH] kvm: apply VM_READ/VM_WRITE checks to all VMA types
  2026-08-03 13:40     ` Sean Christopherson
@ 2026-08-03 17:17       ` Paolo Bonzini
  2026-08-03 22:40         ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2026-08-03 17:17 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-kernel, kvm

On 8/3/26 15:40, Sean Christopherson wrote:
> On Mon, Aug 03, 2026, Paolo Bonzini wrote:
>> On Fri, Jul 31, 2026 at 8:46 PM Sean Christopherson <seanjc@google.com> wrote:
>>>> * if the PTE is present, follow_pfnmap_start() sets args.writable to
>>>>    false and KVM_PFN_ERR_RO_FAULT is returned;
>>>>
>>>> * if no PTE is present, fixup_user_fault(FAULT_FLAG_WRITE) returns
>>>>    -EFAULT after checking vma_permits_fault(), and hva_to_pfn() ends
>>>>    up returning KVM_PFN_ERR_FAULT.
>>>>
>>>> With this patch KVM_PFN_ERR_RO_FAULT is returned uniformly.
>>>
>>> IMO, returning KVM_PFN_ERR_RO_FAULT on a read-only VMA is wrong.  AFAICT, that
>>> behavior for VM_{IO,PFNMAP} was added by commit bd2fae8da794 ("KVM: do not assume
>>> PTE is writable after follow_pfn").  Given that that's the only case where KVM
>>> returns KVM_PFN_ERR_RO_FAULT, I would much prefer to fix that wart and cross our
>>> fingers nothing has come to rely on the behavior in the last ~5 years.
>>
>> We can try, but I'd rather not do that in stable releases (while this
>> patch would be applied there, as a first step towards fixing the DRM
>> issue that Sergio reported
> 
> I don't see how this would help with fixup_user_fault() not actually fixing a
> fault.  Neither returning -EFAULT nor emulating is correct KVM behavior.
> 
>> - it avoids the sashiko issue reported for
>> https://lore.kernel.org/r/20260731160514.1101989-1-pbonzini%40redhat.com/).
> 
> But the issue Sashiko reported is just saying that KVM sometimes does what I'm
> saying KVM should do all the time: return -EFAULT.  Or did I misunderstand that
> one too? :-)

Yes, that's correct.  My point is I'd rather not introduce other changes 
to the !VM_WRITE case in stable releases.

So, the follow_pfnmap_start() patch I posted last Friday is part of the 
fix for Sergio's report; but it introduces one such change---which 
indeed we agree is desirable behavior, but which I'd rather not sneak in 
as part of an unrelated fix for a regression.

So, *this* patch removes a bunch of cases in which hva_to_pfn_remapped() 
is inconsistent, but it leaves RO_FAULT in place for now.  Then 
separately we can, uniformly, do the change from RO_FAULT to -EFAULT.

Paolo

>>>          /*
>>>           * GUP failed.  It could be an inaccessible mapping, a pfnmap one,
>>>           * or the page might be absent.
>>>           */
>>>          if (vma == NULL || unlikely(!(vma->vm_flags & VM_READ)) ||
>>>              ((kfp->flags & FOLL_WRITE) && unlikely(!(vma->vm_flags & VM_WRITE)))) {
>>>                  pfn = KVM_PFN_ERR_FAULT;
>>>          } else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
>>>                  r = hva_to_pfn_remapped(vma, kfp, &pfn);
>>>                  if (r == -EAGAIN)
>>>                          goto retry;
>>>                  if (r < 0)
>>>                          pfn = KVM_PFN_ERR_FAULT;
>>>          } else {
>>>                  pfn = kfp->flags & FOLL_NOWAIT ? KVM_PFN_ERR_NEEDS_IO :
>>>                                                   KVM_PFN_ERR_FAULT;
>>>          }
>>
>> Yes, but I'd do that only in 7.3.
> 



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

* Re: [PATCH] kvm: apply VM_READ/VM_WRITE checks to all VMA types
  2026-08-03 17:17       ` Paolo Bonzini
@ 2026-08-03 22:40         ` Sean Christopherson
  2026-08-04  7:43           ` Paolo Bonzini
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2026-08-03 22:40 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

On Mon, Aug 03, 2026, Paolo Bonzini wrote:
> On 8/3/26 15:40, Sean Christopherson wrote:
> > On Mon, Aug 03, 2026, Paolo Bonzini wrote:
> > > On Fri, Jul 31, 2026 at 8:46 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > > * if the PTE is present, follow_pfnmap_start() sets args.writable to
> > > > >    false and KVM_PFN_ERR_RO_FAULT is returned;
> > > > > 
> > > > > * if no PTE is present, fixup_user_fault(FAULT_FLAG_WRITE) returns
> > > > >    -EFAULT after checking vma_permits_fault(), and hva_to_pfn() ends
> > > > >    up returning KVM_PFN_ERR_FAULT.
> > > > > 
> > > > > With this patch KVM_PFN_ERR_RO_FAULT is returned uniformly.
> > > > 
> > > > IMO, returning KVM_PFN_ERR_RO_FAULT on a read-only VMA is wrong.  AFAICT, that
> > > > behavior for VM_{IO,PFNMAP} was added by commit bd2fae8da794 ("KVM: do not assume
> > > > PTE is writable after follow_pfn").  Given that that's the only case where KVM
> > > > returns KVM_PFN_ERR_RO_FAULT, I would much prefer to fix that wart and cross our
> > > > fingers nothing has come to rely on the behavior in the last ~5 years.
> > > 
> > > We can try, but I'd rather not do that in stable releases (while this
> > > patch would be applied there, as a first step towards fixing the DRM
> > > issue that Sergio reported
> > 
> > I don't see how this would help with fixup_user_fault() not actually fixing a
> > fault.  Neither returning -EFAULT nor emulating is correct KVM behavior.
> > 
> > > - it avoids the sashiko issue reported for
> > > https://lore.kernel.org/r/20260731160514.1101989-1-pbonzini%40redhat.com/).
> > 
> > But the issue Sashiko reported is just saying that KVM sometimes does what I'm
> > saying KVM should do all the time: return -EFAULT.  Or did I misunderstand that
> > one too? :-)
> 
> Yes, that's correct.  My point is I'd rather not introduce other changes to
> the !VM_WRITE case in stable releases.

Yeah, agreed.  But what I don't understand is why this would be sent to stable@
in the first place.

> So, the follow_pfnmap_start() patch I posted last Friday is part of the fix
> for Sergio's report; but it introduces one such change---which indeed we
> agree is desirable behavior, but which I'd rather not sneak in as part of an
> unrelated fix for a regression.
> 
> So, *this* patch removes a bunch of cases in which hva_to_pfn_remapped() is
> inconsistent, but it leaves RO_FAULT in place for now.  Then separately we
> can, uniformly, do the change from RO_FAULT to -EFAULT.
> 
> Paolo
> 
> > > >          /*
> > > >           * GUP failed.  It could be an inaccessible mapping, a pfnmap one,
> > > >           * or the page might be absent.
> > > >           */
> > > >          if (vma == NULL || unlikely(!(vma->vm_flags & VM_READ)) ||
> > > >              ((kfp->flags & FOLL_WRITE) && unlikely(!(vma->vm_flags & VM_WRITE)))) {
> > > >                  pfn = KVM_PFN_ERR_FAULT;
> > > >          } else if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
> > > >                  r = hva_to_pfn_remapped(vma, kfp, &pfn);
> > > >                  if (r == -EAGAIN)
> > > >                          goto retry;
> > > >                  if (r < 0)
> > > >                          pfn = KVM_PFN_ERR_FAULT;
> > > >          } else {
> > > >                  pfn = kfp->flags & FOLL_NOWAIT ? KVM_PFN_ERR_NEEDS_IO :
> > > >                                                   KVM_PFN_ERR_FAULT;
> > > >          }
> > > 
> > > Yes, but I'd do that only in 7.3.
> > 
> 
> 

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

* Re: [PATCH] kvm: apply VM_READ/VM_WRITE checks to all VMA types
  2026-08-03 22:40         ` Sean Christopherson
@ 2026-08-04  7:43           ` Paolo Bonzini
  2026-08-04 13:11             ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2026-08-04  7:43 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-kernel, kvm

On Tue, Aug 4, 2026 at 12:40 AM Sean Christopherson <seanjc@google.com> wrote:
> > > But the issue Sashiko reported is just saying that KVM sometimes does what I'm
> > > saying KVM should do all the time: return -EFAULT.  Or did I misunderstand that
> > > one too? :-)
> >
> > Yes, that's correct.  My point is I'd rather not introduce other changes to
> > the !VM_WRITE case in stable releases.
>
> Yeah, agreed.  But what I don't understand is why this would be sent to stable@
> in the first place.

Because the patch
https://lore.kernel.org/kvm/20260731160514.1101989-1-pbonzini@redhat.com/T/
needs to be sent to stable@, and this one fixes a problem in that
other patch. So this one needs to come first.

Paolo


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

* Re: [PATCH] kvm: apply VM_READ/VM_WRITE checks to all VMA types
  2026-08-04  7:43           ` Paolo Bonzini
@ 2026-08-04 13:11             ` Sean Christopherson
  0 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2026-08-04 13:11 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

On Tue, Aug 04, 2026, Paolo Bonzini wrote:
> On Tue, Aug 4, 2026 at 12:40 AM Sean Christopherson <seanjc@google.com> wrote:
> > > > But the issue Sashiko reported is just saying that KVM sometimes does what I'm
> > > > saying KVM should do all the time: return -EFAULT.  Or did I misunderstand that
> > > > one too? :-)
> > >
> > > Yes, that's correct.  My point is I'd rather not introduce other changes to
> > > the !VM_WRITE case in stable releases.
> >
> > Yeah, agreed.  But what I don't understand is why this would be sent to stable@
> > in the first place.
> 
> Because the patch
> https://lore.kernel.org/kvm/20260731160514.1101989-1-pbonzini@redhat.com/T/
> needs to be sent to stable@, and this one fixes a problem in that
> other patch. So this one needs to come first.

Oooh, that makes sense.  Thanks for being patient.

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

end of thread, other threads:[~2026-08-04 13:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-31 17:58 [PATCH] kvm: apply VM_READ/VM_WRITE checks to all VMA types Paolo Bonzini
2026-07-31 18:46 ` Sean Christopherson
2026-08-03  5:54   ` Paolo Bonzini
2026-08-03 13:40     ` Sean Christopherson
2026-08-03 17:17       ` Paolo Bonzini
2026-08-03 22:40         ` Sean Christopherson
2026-08-04  7:43           ` Paolo Bonzini
2026-08-04 13:11             ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox