[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