[PATCH] KVM: x86/hyperv: do not overwrite hc->ingpa for slow SIGNAL_EVENT hypercall

[PATCH] KVM: x86/hyperv: do not overwrite hc->ingpa for slow SIGNAL_EVENT hypercall

[Paolo Bonzini]

When a guest makes a slow HVCALL_SIGNAL_EVENT hypercall with a connection ID
that is valid in userspace but not registered in the kernel conn_to_evt,
kvm_hvcall_signal_event() reads the connection ID and overwrites hc->ingpa.
However, hc->param still signals that the hypercall was a slow one, and
userspace will then treat the connection ID as an address.

Cc: stable@vger.kernel.org
Fixes: bd38b32053eb ("KVM: hyper-v: Collect hypercall params into struct")
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        originally reported by sashiko at
        https://lore.kernel.org/kvm/20260918083322.C87F11F000FF@smtp.kernel.org/,
        but the issue is preexisting and unrelated to the patch that was being
        reviewed.

 arch/x86/kvm/hyperv.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 9f5adcd26cba..e3a8e8236230 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -2501,6 +2501,7 @@ static int kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *h
 {
 	struct kvm_hv *hv = to_kvm_hv(vcpu->kvm);
 	struct eventfd_ctx *eventfd;
+	u64 conn_id;
 	int ret;
 
 	ret = kvm_hv_hypercall_check_params(vcpu, hc);
@@ -2511,14 +2512,16 @@ static int kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *h
 		int ret;
 		gpa_t gpa = hc->ingpa;
 
-		if ((gpa & (__alignof__(hc->ingpa) - 1)) ||
-		    offset_in_page(gpa) + sizeof(hc->ingpa) > PAGE_SIZE)
+		if ((gpa & (__alignof__(conn_id) - 1)) ||
+		    offset_in_page(gpa) + sizeof(conn_id) > PAGE_SIZE)
 			return HV_STATUS_INVALID_ALIGNMENT;
 
 		ret = kvm_vcpu_read_guest(vcpu, gpa,
-					  &hc->ingpa, sizeof(hc->ingpa));
+					  &conn_id, sizeof(conn_id));
 		if (ret < 0)
 			return HV_STATUS_INVALID_ALIGNMENT;
+	} else {
+		conn_id = hc->ingpa;
 	}
 
 	/*
@@ -2526,15 +2529,15 @@ static int kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *h
 	 * have no use for it, and in all known usecases it is zero, so just
 	 * report lookup failure if it isn't.
 	 */
-	if (hc->ingpa & 0xffff00000000ULL)
+	if (conn_id & 0xffff00000000ULL)
 		return HV_STATUS_INVALID_PORT_ID;
 	/* remaining bits are reserved-zero */
-	if (hc->ingpa & ~KVM_HYPERV_CONN_ID_MASK)
+	if (conn_id & ~KVM_HYPERV_CONN_ID_MASK)
 		return HV_STATUS_INVALID_HYPERCALL_INPUT;
 
 	/* the eventfd is protected by vcpu->kvm->srcu, but conn_to_evt isn't */
 	rcu_read_lock();
-	eventfd = idr_find(&hv->conn_to_evt, hc->ingpa);
+	eventfd = idr_find(&hv->conn_to_evt, conn_id);
 	rcu_read_unlock();
 	if (!eventfd)
 		return HV_STATUS_INVALID_PORT_ID;
-- 
2.52.0

Re: [PATCH] KVM: x86/hyperv: do not overwrite hc->ingpa for slow SIGNAL_EVENT hypercall

[Sean Christopherson]

On Fri, Sep 18, 2026, Paolo Bonzini wrote:
> When a guest makes a slow HVCALL_SIGNAL_EVENT hypercall with a connection ID
> that is valid in userspace but not registered in the kernel conn_to_evt,
> kvm_hvcall_signal_event() reads the connection ID and overwrites hc->ingpa.
> However, hc->param still signals that the hypercall was a slow one, and
> userspace will then treat the connection ID as an address.
> 
> Cc: stable@vger.kernel.org
> Fixes: bd38b32053eb ("KVM: hyper-v: Collect hypercall params into struct")
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         originally reported by sashiko at
>         https://lore.kernel.org/kvm/20260918083322.C87F11F000FF@smtp.kernel.org/,
>         but the issue is preexisting and unrelated to the patch that was being
>         reviewed.
> 
>  arch/x86/kvm/hyperv.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 9f5adcd26cba..e3a8e8236230 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -2501,6 +2501,7 @@ static int kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *h
>  {
>  	struct kvm_hv *hv = to_kvm_hv(vcpu->kvm);
>  	struct eventfd_ctx *eventfd;
> +	u64 conn_id;
>  	int ret;

This doesn't apply to any branch I can find, and there is some unnecessary variable
shadowing going on here as well.

The actual change looks good, but the diff is wonky.

>  	ret = kvm_hv_hypercall_check_params(vcpu, hc);
> @@ -2511,14 +2512,16 @@ static int kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *h
>  		int ret;
>  		gpa_t gpa = hc->ingpa;
>  
> -		if ((gpa & (__alignof__(hc->ingpa) - 1)) ||
> -		    offset_in_page(gpa) + sizeof(hc->ingpa) > PAGE_SIZE)
> +		if ((gpa & (__alignof__(conn_id) - 1)) ||
> +		    offset_in_page(gpa) + sizeof(conn_id) > PAGE_SIZE)
>  			return HV_STATUS_INVALID_ALIGNMENT;
>  
>  		ret = kvm_vcpu_read_guest(vcpu, gpa,
> -					  &hc->ingpa, sizeof(hc->ingpa));
> +					  &conn_id, sizeof(conn_id));
>  		if (ret < 0)
>  			return HV_STATUS_INVALID_ALIGNMENT;
> +	} else {
> +		conn_id = hc->ingpa;
>  	}
>  
>  	/*
> @@ -2526,15 +2529,15 @@ static int kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *h
>  	 * have no use for it, and in all known usecases it is zero, so just
>  	 * report lookup failure if it isn't.
>  	 */
> -	if (hc->ingpa & 0xffff00000000ULL)
> +	if (conn_id & 0xffff00000000ULL)
>  		return HV_STATUS_INVALID_PORT_ID;
>  	/* remaining bits are reserved-zero */
> -	if (hc->ingpa & ~KVM_HYPERV_CONN_ID_MASK)
> +	if (conn_id & ~KVM_HYPERV_CONN_ID_MASK)
>  		return HV_STATUS_INVALID_HYPERCALL_INPUT;
>  
>  	/* the eventfd is protected by vcpu->kvm->srcu, but conn_to_evt isn't */
>  	rcu_read_lock();
> -	eventfd = idr_find(&hv->conn_to_evt, hc->ingpa);
> +	eventfd = idr_find(&hv->conn_to_evt, conn_id);
>  	rcu_read_unlock();
>  	if (!eventfd)
>  		return HV_STATUS_INVALID_PORT_ID;
> -- 
> 2.52.0
> 

Re: [PATCH] KVM: x86/hyperv: do not overwrite hc->ingpa for slow SIGNAL_EVENT hypercall

[Paolo Bonzini]

On Tue, Sep 22, 2026 at 12:52 AM Sean Christopherson <seanjc@google.com> wrote:
> > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> > index 9f5adcd26cba..e3a8e8236230 100644
> > --- a/arch/x86/kvm/hyperv.c
> > +++ b/arch/x86/kvm/hyperv.c
> > @@ -2501,6 +2501,7 @@ static int kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *h
> >  {
> >       struct kvm_hv *hv = to_kvm_hv(vcpu->kvm);
> >       struct eventfd_ctx *eventfd;
> > +     u64 conn_id;
> >       int ret;
>
> This doesn't apply to any branch I can find, and there is some unnecessary variable
> shadowing going on here as well.
>
> The actual change looks good, but the diff is wonky.

You're right, I probably generated this from the wrong branch (or
machine). The right patch is at
https://lore.kernel.org/kvm/20260918135030.171564-2-pbonzini@redhat.com/.

(BTW, I would like to talk about memory attributes and kvm-userfault
next Wednesday... and whether those are two different things at all).

Paolo


> >       ret = kvm_hv_hypercall_check_params(vcpu, hc);
> > @@ -2511,14 +2512,16 @@ static int kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *h
> >               int ret;
> >               gpa_t gpa = hc->ingpa;
> >
> > -             if ((gpa & (__alignof__(hc->ingpa) - 1)) ||
> > -                 offset_in_page(gpa) + sizeof(hc->ingpa) > PAGE_SIZE)
> > +             if ((gpa & (__alignof__(conn_id) - 1)) ||
> > +                 offset_in_page(gpa) + sizeof(conn_id) > PAGE_SIZE)
> >                       return HV_STATUS_INVALID_ALIGNMENT;
> >
> >               ret = kvm_vcpu_read_guest(vcpu, gpa,
> > -                                       &hc->ingpa, sizeof(hc->ingpa));
> > +                                       &conn_id, sizeof(conn_id));
> >               if (ret < 0)
> >                       return HV_STATUS_INVALID_ALIGNMENT;
> > +     } else {
> > +             conn_id = hc->ingpa;
> >       }
> >
> >       /*
> > @@ -2526,15 +2529,15 @@ static int kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *h
> >        * have no use for it, and in all known usecases it is zero, so just
> >        * report lookup failure if it isn't.
> >        */
> > -     if (hc->ingpa & 0xffff00000000ULL)
> > +     if (conn_id & 0xffff00000000ULL)
> >               return HV_STATUS_INVALID_PORT_ID;
> >       /* remaining bits are reserved-zero */
> > -     if (hc->ingpa & ~KVM_HYPERV_CONN_ID_MASK)
> > +     if (conn_id & ~KVM_HYPERV_CONN_ID_MASK)
> >               return HV_STATUS_INVALID_HYPERCALL_INPUT;
> >
> >       /* the eventfd is protected by vcpu->kvm->srcu, but conn_to_evt isn't */
> >       rcu_read_lock();
> > -     eventfd = idr_find(&hv->conn_to_evt, hc->ingpa);
> > +     eventfd = idr_find(&hv->conn_to_evt, conn_id);
> >       rcu_read_unlock();
> >       if (!eventfd)
> >               return HV_STATUS_INVALID_PORT_ID;
> > --
> > 2.52.0
> >
>

Re: [PATCH] KVM: x86/hyperv: do not overwrite hc->ingpa for slow SIGNAL_EVENT hypercall

[Sean Christopherson]

+James

On Tue, Sep 22, 2026, Paolo Bonzini wrote:
> On Tue, Sep 22, 2026 at 12:52 AM Sean Christopherson <seanjc@google.com> wrote:
> > > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> > > index 9f5adcd26cba..e3a8e8236230 100644
> > > --- a/arch/x86/kvm/hyperv.c
> > > +++ b/arch/x86/kvm/hyperv.c
> > > @@ -2501,6 +2501,7 @@ static int kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *h
> > >  {
> > >       struct kvm_hv *hv = to_kvm_hv(vcpu->kvm);
> > >       struct eventfd_ctx *eventfd;
> > > +     u64 conn_id;
> > >       int ret;
> >
> > This doesn't apply to any branch I can find, and there is some unnecessary variable
> > shadowing going on here as well.
> >
> > The actual change looks good, but the diff is wonky.
> 
> You're right, I probably generated this from the wrong branch (or
> machine). The right patch is at
> https://lore.kernel.org/kvm/20260918135030.171564-2-pbonzini@redhat.com/.
> 
> (BTW, I would like to talk about memory attributes and kvm-userfault
> next Wednesday... and whether those are two different things at all).

By "next" Wednesday, do you mean the 23rd or the 30th?  I want to make sure James
is pulled into the discussion as he know a lot more than I do with respect to
Google's use cases for KVM Userfault.

Re: [PATCH] KVM: x86/hyperv: do not overwrite hc->ingpa for slow SIGNAL_EVENT hypercall

[Paolo Bonzini]

On Tue, Sep 22, 2026 at 1:01 AM Sean Christopherson <seanjc@google.com> wrote:
>
> +James
>
> On Tue, Sep 22, 2026, Paolo Bonzini wrote:
> > (BTW, I would like to talk about memory attributes and kvm-userfault
> > next Wednesday... and whether those are two different things at all).
>
> By "next" Wednesday, do you mean the 23rd or the 30th?  I want to make sure James
> is pulled into the discussion as he know a lot more than I do with respect to
> Google's use cases for KVM Userfault.

Either is fine, of course 23rd is better if there are no other topics planned.

Paolo

Re: [PATCH] KVM: x86/hyperv: do not overwrite hc->ingpa for slow SIGNAL_EVENT hypercall

[Sean Christopherson]

+James for real this time.

On Tue, Sep 22, 2026, Paolo Bonzini wrote:
> On Tue, Sep 22, 2026 at 1:01 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > +James
> >
> > On Tue, Sep 22, 2026, Paolo Bonzini wrote:
> > > (BTW, I would like to talk about memory attributes and kvm-userfault
> > > next Wednesday... and whether those are two different things at all).
> >
> > By "next" Wednesday, do you mean the 23rd or the 30th?  I want to make sure James
> > is pulled into the discussion as he know a lot more than I do with respect to
> > Google's use cases for KVM Userfault.
> 
> Either is fine, of course 23rd is better if there are no other topics planned.

James, how are you feeling about an early morning PUCK in the near future?

Re: [PATCH] KVM: x86/hyperv: do not overwrite hc->ingpa for slow SIGNAL_EVENT hypercall

[James Houghton]

On Mon, Sep 21, 2026 at 4:11 PM Sean Christopherson <seanjc@google.com> wrote:
>
> +James for real this time.
>
> On Tue, Sep 22, 2026, Paolo Bonzini wrote:
> > On Tue, Sep 22, 2026 at 1:01 AM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > +James
> > >
> > > On Tue, Sep 22, 2026, Paolo Bonzini wrote:
> > > > (BTW, I would like to talk about memory attributes and kvm-userfault
> > > > next Wednesday... and whether those are two different things at all).
> > >
> > > By "next" Wednesday, do you mean the 23rd or the 30th?  I want to make sure James
> > > is pulled into the discussion as he know a lot more than I do with respect to
> > > Google's use cases for KVM Userfault.
> >
> > Either is fine, of course 23rd is better if there are no other topics planned.
>
> James, how are you feeling about an early morning PUCK in the near future?

No problems here; the 23rd is fine. The only day I can't do in the
near-ish future is Oct 14. Thanks!

Re: [PATCH] KVM: x86/hyperv: do not overwrite hc->ingpa for slow SIGNAL_EVENT hypercall

[Paolo Bonzini]

On 9/22/26 01:23, James Houghton wrote:
>> James, how are you feeling about an early morning PUCK in the near future?
> No problems here; the 23rd is fine. The only day I can't do in the
> near-ish future is Oct 14. Thanks!

Great.  To sum up what I would like to understand, this is it:

1) the overlap between memory protection attributes and kvm-userfault. 
It seems to me that they are almost the same, but I may be wrong.

In particular, they are almost the same in that they are both 
stop-and-retry interfaces, unlike userfaultfd which hides the 
stop-and-retry behind a page fault and a stopped thread.

2) what strategy you have implemented, or you have in mind, to handle 
stop-and-retry for code that does many consecutive memory accesses.  The 
main one is nested vmentry/vmexit.  I have here some mostly untested 
code that splits them into prepare/commit/cancel phases, but I haven't 
dared posting it.

(Secondarily, we have a TDX-sized dependency between planes, memory 
attributes, VBS, and kvm-userfault.  That is probably something worth 
discussing too, sooner or later).

Paolo

Re: [PATCH] KVM: x86/hyperv: do not overwrite hc->ingpa for slow SIGNAL_EVENT hypercall

[James Houghton]

On Mon, Sep 21, 2026 at 4:33 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 9/22/26 01:23, James Houghton wrote:
> >> James, how are you feeling about an early morning PUCK in the near future?
> > No problems here; the 23rd is fine. The only day I can't do in the
> > near-ish future is Oct 14. Thanks!
>
> Great.  To sum up what I would like to understand, this is it:
>
> 1) the overlap between memory protection attributes and kvm-userfault.
> It seems to me that they are almost the same, but I may be wrong.
>
> In particular, they are almost the same in that they are both
> stop-and-retry interfaces, unlike userfaultfd which hides the
> stop-and-retry behind a page fault and a stopped thread.

Hi Paolo,

They are very similar, yes. But I am unsure what the new memory
attribute work is motivated by (maybe I missed it? I don't see it in
the cover letters I checked).

KVM Userfault was motivated by post-copy live migration (for VMs with
guest_memfd memslots and as an optimization for userfaultfd-based
post-copy with conventional VMs). KVM Userfault's interface reflects
this; it uses a userspace-modifiable bitmap to track state, which has
predictable memory overhead and is fast to modify. If memory
attributes are meant to be used for post-copy as well, we'll want a
bitmap-based interface for userspace and for tracking in KVM.

Other than the interface, KVM Userfault was intentionally limited to
only EPT/NPT faults because it is always "safe" for those to return to
userspace with -EFAULT and the bad address. This means that
implementing post-copy completely requires *also* using userfaultfd
(or something else) for the other guest memory access cases. Memory
protection attributes include many of the other important
(guest-initiated) cases, so post-copy could maybe be implemented
without userfaultfd. Great!

I was assuming that these new checks would be (1) difficult to
maintain and (2) potentially cause issues with instruction emulation
(or any case where unwinding and replaying is challenging).

> 2) what strategy you have implemented, or you have in mind, to handle
> stop-and-retry for code that does many consecutive memory accesses.  The
> main one is nested vmentry/vmexit.  I have here some mostly untested
> code that splits them into prepare/commit/cancel phases, but I haven't
> dared posting it.

Ah exactly... :) This is why userfaultfd remained in the picture for a
complete post-copy implementation with KVM Userfault. From my
perspective, the only way around using userfaultfd is to add support
for nested vmentry/vmexit going back out to userspace and coming back
in, like the phases you have.

"KVM Demand Paging" in Google's downstream kernel has essentially the
same checks that you have in the memory attribute series. It uses a
netlink socket for the long flows (and waits for userspace to mark the
page as ready). Perhaps there's an argument that this would be better
than making the long flows support exiting to userspace? But I doubt
it.

> (Secondarily, we have a TDX-sized dependency between planes, memory
> attributes, VBS, and kvm-userfault.  That is probably something worth
> discussing too, sooner or later).

Yes I want to hear more, I'm not sure of all the context here. :)

Thanks!

James