arch/x86/kvm/hyperv.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
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.0On 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
>
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
> >
>
+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.
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
+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?
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!
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
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
© 2016 - 2026 Red Hat, Inc.