[PATCH] KVM: x86: hyper-v: Bound the bank index in hv_is_vp_in_sparse_set()

[PATCH] KVM: x86: hyper-v: Bound the bank index in hv_is_vp_in_sparse_set()

[Hyunwoo Kim]

hv_is_vp_in_sparse_set() uses valid_bit_nr, i.e. vp_id divided by
HV_VCPUS_PER_SPARSE_BANK, as the test_bit() index into
valid_bank_mask. valid_bank_mask is a single u64 and a sparse vCPU
set holds at most HV_MAX_SPARSE_VCPU_BANKS banks, so valid_bit_nr
must be less than HV_MAX_SPARSE_VCPU_BANKS.

The caller in kvm_hv_send_ipi_to_many() passes kvm_hv_get_vpindex(),
which is below KVM_MAX_VCPUS and therefore always within that bound.
The L2 direct flush branch in kvm_hv_flush_tlb(), however, passes
hv_v->nested.vp_id, copied verbatim from the enlightened VMCS
without any bounds check, so valid_bit_nr can reach
HV_MAX_SPARSE_VCPU_BANKS or more and test_bit() then reads beyond
valid_bank_mask.

Return false before the test_bit() when valid_bit_nr is not below
HV_MAX_SPARSE_VCPU_BANKS, since such a VP cannot be present in the
set.

Cc: stable@vger.kernel.org
Fixes: c58a318f6090 ("KVM: x86: hyper-v: L2 TLB flush")
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 arch/x86/kvm/hyperv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 4438ecac9a89..d8782cb7ba02 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1839,6 +1839,10 @@ static bool hv_is_vp_in_sparse_set(u32 vp_id, u64 valid_bank_mask, u64 sparse_ba
 	int valid_bit_nr = vp_id / HV_VCPUS_PER_SPARSE_BANK;
 	unsigned long sbank;
 
+	/* A bank index beyond the mask can't be set, the VP isn't in the set. */
+	if (valid_bit_nr >= HV_MAX_SPARSE_VCPU_BANKS)
+		return false;
+
 	if (!test_bit(valid_bit_nr, (unsigned long *)&valid_bank_mask))
 		return false;
 
-- 
2.43.0

Re: [PATCH] KVM: x86: hyper-v: Bound the bank index in hv_is_vp_in_sparse_set()

[Vitaly Kuznetsov]

Hyunwoo Kim <imv4bel@gmail.com> writes:

> hv_is_vp_in_sparse_set() uses valid_bit_nr, i.e. vp_id divided by
> HV_VCPUS_PER_SPARSE_BANK, as the test_bit() index into
> valid_bank_mask. valid_bank_mask is a single u64 and a sparse vCPU
> set holds at most HV_MAX_SPARSE_VCPU_BANKS banks, so valid_bit_nr
> must be less than HV_MAX_SPARSE_VCPU_BANKS.
>
> The caller in kvm_hv_send_ipi_to_many() passes kvm_hv_get_vpindex(),
> which is below KVM_MAX_VCPUS and therefore always within that bound.
> The L2 direct flush branch in kvm_hv_flush_tlb(), however, passes
> hv_v->nested.vp_id, copied verbatim from the enlightened VMCS
> without any bounds check, so valid_bit_nr can reach
> HV_MAX_SPARSE_VCPU_BANKS or more and test_bit() then reads beyond
> valid_bank_mask.
>
> Return false before the test_bit() when valid_bit_nr is not below
> HV_MAX_SPARSE_VCPU_BANKS, since such a VP cannot be present in the
> set.
>
> Cc: stable@vger.kernel.org
> Fixes: c58a318f6090 ("KVM: x86: hyper-v: L2 TLB flush")
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  arch/x86/kvm/hyperv.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 4438ecac9a89..d8782cb7ba02 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1839,6 +1839,10 @@ static bool hv_is_vp_in_sparse_set(u32 vp_id, u64 valid_bank_mask, u64 sparse_ba
>  	int valid_bit_nr = vp_id / HV_VCPUS_PER_SPARSE_BANK;
>  	unsigned long sbank;
>  
> +	/* A bank index beyond the mask can't be set, the VP isn't in the set. */
> +	if (valid_bit_nr >= HV_MAX_SPARSE_VCPU_BANKS)
> +		return false;
> +
>  	if (!test_bit(valid_bit_nr, (unsigned long *)&valid_bank_mask))
>  		return false;

I think the concern is valid, so

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

what I'm not sure about if we should also deliberately crash the VM
which does such a hypercall. This way it would be easier to find buggy
L1s but given that they are most likely Windows, we need to do some
tests to see if this is not actually happening today (e.g. Hyper-V usign
VP_ID or '-1' for something). Let's have this as a future TODO item.

-- 
Vitaly

Re: [PATCH] KVM: x86: hyper-v: Bound the bank index in hv_is_vp_in_sparse_set()

[Sean Christopherson]

On Mon, Jun 08, 2026, Vitaly Kuznetsov wrote:
> Hyunwoo Kim <imv4bel@gmail.com> writes:
> 
> > hv_is_vp_in_sparse_set() uses valid_bit_nr, i.e. vp_id divided by
> > HV_VCPUS_PER_SPARSE_BANK, as the test_bit() index into
> > valid_bank_mask. valid_bank_mask is a single u64 and a sparse vCPU
> > set holds at most HV_MAX_SPARSE_VCPU_BANKS banks, so valid_bit_nr
> > must be less than HV_MAX_SPARSE_VCPU_BANKS.
> >
> > The caller in kvm_hv_send_ipi_to_many() passes kvm_hv_get_vpindex(),
> > which is below KVM_MAX_VCPUS and therefore always within that bound.
> > The L2 direct flush branch in kvm_hv_flush_tlb(), however, passes
> > hv_v->nested.vp_id, copied verbatim from the enlightened VMCS
> > without any bounds check, so valid_bit_nr can reach
> > HV_MAX_SPARSE_VCPU_BANKS or more and test_bit() then reads beyond
> > valid_bank_mask.
> >
> > Return false before the test_bit() when valid_bit_nr is not below
> > HV_MAX_SPARSE_VCPU_BANKS, since such a VP cannot be present in the
> > set.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: c58a318f6090 ("KVM: x86: hyper-v: L2 TLB flush")
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > ---
> >  arch/x86/kvm/hyperv.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> > index 4438ecac9a89..d8782cb7ba02 100644
> > --- a/arch/x86/kvm/hyperv.c
> > +++ b/arch/x86/kvm/hyperv.c
> > @@ -1839,6 +1839,10 @@ static bool hv_is_vp_in_sparse_set(u32 vp_id, u64 valid_bank_mask, u64 sparse_ba
> >  	int valid_bit_nr = vp_id / HV_VCPUS_PER_SPARSE_BANK;
> >  	unsigned long sbank;
> >  
> > +	/* A bank index beyond the mask can't be set, the VP isn't in the set. */
> > +	if (valid_bit_nr >= HV_MAX_SPARSE_VCPU_BANKS)
> > +		return false;
> > +
> >  	if (!test_bit(valid_bit_nr, (unsigned long *)&valid_bank_mask))
> >  		return false;
> 
> I think the concern is valid, so

Yeah, easy to trigger with KASAN and:

diff --git tools/testing/selftests/kvm/x86/hyperv_evmcs.c tools/testing/selftests/kvm/x86/hyperv_evmcs.c
index c7fa114aee20..0cf5f891a20d 100644
--- tools/testing/selftests/kvm/x86/hyperv_evmcs.c
+++ tools/testing/selftests/kvm/x86/hyperv_evmcs.c
@@ -59,6 +59,10 @@ void l2_guest_code(void)
        vmcall();
        rdmsr_from_l2(MSR_GS_BASE); /* intercepted */
 
+       asm volatile ("movq %0, %%xmm0" :: "r"(-1ull));
+       hyperv_hypercall(HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE | HV_HYPERCALL_FAST_BIT, 0x0,
+                        HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES);
+
        /* L2 TLB flush tests */
        hyperv_hypercall(HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE | HV_HYPERCALL_FAST_BIT, 0x0,
                         HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES | HV_FLUSH_ALL_PROCESSORS);
@@ -117,7 +121,7 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
        current_evmcs->partition_assist_page = hv_pages->partition_assist_gpa;
        current_evmcs->hv_enlightenments_control.nested_flush_hypercall = 1;
        current_evmcs->hv_vm_id = 1;
-       current_evmcs->hv_vp_id = 1;
+       current_evmcs->hv_vp_id = -1;
        current_vp_assist->nested_control.features.directhypercall = 1;
        *(u32 *)(hv_pages->partition_assist) = 0;
 


> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> 
> what I'm not sure about if we should also deliberately crash the VM
> which does such a hypercall. This way it would be easier to find buggy
> L1s but given that they are most likely Windows, we need to do some
> tests to see if this is not actually happening today (e.g. Hyper-V usign
> VP_ID or '-1' for something). Let's have this as a future TODO item.

+1