[PATCH 0/5] KVM: x86: Hyper-V SEND_IPI fix and partial testcase

[PATCH 0/5] KVM: x86: Hyper-V SEND_IPI fix and partial testcase

[Sean Christopherson]

Fix a NULL pointer deref due to exposing Hyper-V enlightments to a guest
without an in-kernel local APIC (found by syzkaller, highly unlikely to
affect any "real" VMMs).  Expand the Hyper-V CPUID test to verify that KVM
doesn't incorrectly advertise support.

Sean Christopherson (5):
  KVM: x86: Reject Hyper-V's SEND_IPI hypercalls if local APIC isn't
    in-kernel
  KVM: selftests: Mark test_hv_cpuid_e2big() static in Hyper-V CPUID
    test
  KVM: selftests: Explicitly free CPUID array at end of Hyper-V CPUID
    test
  KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper
  KVM: selftests: Add CPUID tests for Hyper-V features that need
    in-kernel APIC

 arch/x86/kvm/hyperv.c                         |  6 ++-
 .../selftests/kvm/x86_64/hyperv_cpuid.c       | 41 ++++++++++++-------
 2 files changed, 31 insertions(+), 16 deletions(-)


base-commit: a5546c2f0dc4f84727a4bb8a91633917929735f5
-- 
2.47.1.688.g23fc6f90ad-goog

[PATCH 1/5] KVM: x86: Reject Hyper-V's SEND_IPI hypercalls if local APIC isn't in-kernel

[Sean Christopherson]

Advertise support for Hyper-V's SEND_IPI and SEND_IPI_EX hypercalls if and
only if the local API is emulated/virtualized by KVM, and explicitly reject
said hypercalls if the local APIC is emulated in userspace, i.e. don't rely
on userspace to opt-in to KVM_CAP_HYPERV_ENFORCE_CPUID.

Rejecting SEND_IPI and SEND_IPI_EX fixes a NULL-pointer dereference if
Hyper-V enlightenments are exposed to the guest without an in-kernel local
APIC:

  dump_stack+0xbe/0xfd
  __kasan_report.cold+0x34/0x84
  kasan_report+0x3a/0x50
  __apic_accept_irq+0x3a/0x5c0
  kvm_hv_send_ipi.isra.0+0x34e/0x820
  kvm_hv_hypercall+0x8d9/0x9d0
  kvm_emulate_hypercall+0x506/0x7e0
  __vmx_handle_exit+0x283/0xb60
  vmx_handle_exit+0x1d/0xd0
  vcpu_enter_guest+0x16b0/0x24c0
  vcpu_run+0xc0/0x550
  kvm_arch_vcpu_ioctl_run+0x170/0x6d0
  kvm_vcpu_ioctl+0x413/0xb20
  __se_sys_ioctl+0x111/0x160
  do_syscal1_64+0x30/0x40
  entry_SYSCALL_64_after_hwframe+0x67/0xd1

Note, checking the sending vCPU is sufficient, as the per-VM irqchip_mode
can't be modified after vCPUs are created, i.e. if one vCPU has an
in-kernel local APIC, then all vCPUs have an in-kernel local APIC.

Reported-by: Dongjie Zou <zoudongjie@huawei.com>
Fixes: 214ff83d4473 ("KVM: x86: hyperv: implement PV IPI send hypercalls")
Fixes: 2bc39970e932 ("x86/kvm/hyper-v: Introduce KVM_GET_SUPPORTED_HV_CPUID")
Cc: stable@vger.kernel
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/hyperv.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 4f0a94346d00..44c88537448c 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -2226,6 +2226,9 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 	u32 vector;
 	bool all_cpus;
 
+	if (!lapic_in_kernel(vcpu))
+		return HV_STATUS_INVALID_HYPERCALL_INPUT;
+
 	if (hc->code == HVCALL_SEND_IPI) {
 		if (!hc->fast) {
 			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi,
@@ -2852,7 +2855,8 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 			ent->eax |= HV_X64_REMOTE_TLB_FLUSH_RECOMMENDED;
 			ent->eax |= HV_X64_APIC_ACCESS_RECOMMENDED;
 			ent->eax |= HV_X64_RELAXED_TIMING_RECOMMENDED;
-			ent->eax |= HV_X64_CLUSTER_IPI_RECOMMENDED;
+			if (!vcpu || lapic_in_kernel(vcpu))
+				ent->eax |= HV_X64_CLUSTER_IPI_RECOMMENDED;
 			ent->eax |= HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED;
 			if (evmcs_ver)
 				ent->eax |= HV_X64_ENLIGHTENED_VMCS_RECOMMENDED;
-- 
2.47.1.688.g23fc6f90ad-goog

[PATCH 2/5] KVM: selftests: Mark test_hv_cpuid_e2big() static in Hyper-V CPUID test

[Sean Christopherson]

Make the Hyper-V CPUID test's local helper test_hv_cpuid_e2big() static,
it's not used outside of the test (and isn't intended to be).

Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 4f5881d4ef66..9a0fcc713350 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -111,7 +111,7 @@ static void test_hv_cpuid(const struct kvm_cpuid2 *hv_cpuid_entries,
 	}
 }
 
-void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
+static void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
 {
 	static struct kvm_cpuid2 cpuid = {.nent = 0};
 	int ret;
-- 
2.47.1.688.g23fc6f90ad-goog

[PATCH 3/5] KVM: selftests: Explicitly free CPUID array at end of Hyper-V CPUID test

[Sean Christopherson]

Explicitly free the array of CPUID entries at the end of the Hyper-V CPUID
test, mainly in anticipation of moving management of the array into the
main test helper.

Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 9a0fcc713350..09f9874d7705 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -164,6 +164,7 @@ int main(int argc, char *argv[])
 
 	hv_cpuid_entries = kvm_get_supported_hv_cpuid();
 	test_hv_cpuid(hv_cpuid_entries, kvm_cpu_has(X86_FEATURE_VMX));
+	free((void *)hv_cpuid_entries);
 
 out:
 	kvm_vm_free(vm);
-- 
2.47.1.688.g23fc6f90ad-goog

[PATCH 4/5] KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper

[Sean Christopherson]

Allocate, get, and free the CPUID array in the Hyper-V CPUID test in the
test's core helper, instead of copy+pasting code at each call site.  In
addition to deduplicating a small amount of code, restricting visibility
of the array to a single invocation of the core test prevents "leaking" an
array across test cases.  Passing in @vcpu to the helper will also allow
pivoting on VM-scoped information without needing to pass more booleans,
e.g. to conditionally assert on features that require an in-kernel APIC.

Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../selftests/kvm/x86_64/hyperv_cpuid.c       | 25 ++++++++-----------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 09f9874d7705..90c44765d584 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -41,13 +41,18 @@ static bool smt_possible(void)
 	return res;
 }
 
-static void test_hv_cpuid(const struct kvm_cpuid2 *hv_cpuid_entries,
-			  bool evmcs_expected)
+static void test_hv_cpuid(struct kvm_vcpu *vcpu, bool evmcs_expected)
 {
+	const struct kvm_cpuid2 *hv_cpuid_entries;
 	int i;
 	int nent_expected = 10;
 	u32 test_val;
 
+	if (vcpu)
+		hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu);
+	else
+		hv_cpuid_entries = kvm_get_supported_hv_cpuid();
+
 	TEST_ASSERT(hv_cpuid_entries->nent == nent_expected,
 		    "KVM_GET_SUPPORTED_HV_CPUID should return %d entries"
 		    " (returned %d)",
@@ -109,6 +114,7 @@ static void test_hv_cpuid(const struct kvm_cpuid2 *hv_cpuid_entries,
 		 *	entry->edx);
 		 */
 	}
+	free((void *)hv_cpuid_entries);
 }
 
 static void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
@@ -129,7 +135,6 @@ static void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
 int main(int argc, char *argv[])
 {
 	struct kvm_vm *vm;
-	const struct kvm_cpuid2 *hv_cpuid_entries;
 	struct kvm_vcpu *vcpu;
 
 	TEST_REQUIRE(kvm_has_cap(KVM_CAP_HYPERV_CPUID));
@@ -138,10 +143,7 @@ int main(int argc, char *argv[])
 
 	/* Test vCPU ioctl version */
 	test_hv_cpuid_e2big(vm, vcpu);
-
-	hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu);
-	test_hv_cpuid(hv_cpuid_entries, false);
-	free((void *)hv_cpuid_entries);
+	test_hv_cpuid(vcpu, false);
 
 	if (!kvm_cpu_has(X86_FEATURE_VMX) ||
 	    !kvm_has_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
@@ -149,9 +151,7 @@ int main(int argc, char *argv[])
 		goto do_sys;
 	}
 	vcpu_enable_evmcs(vcpu);
-	hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu);
-	test_hv_cpuid(hv_cpuid_entries, true);
-	free((void *)hv_cpuid_entries);
+	test_hv_cpuid(vcpu, true);
 
 do_sys:
 	/* Test system ioctl version */
@@ -161,10 +161,7 @@ int main(int argc, char *argv[])
 	}
 
 	test_hv_cpuid_e2big(vm, NULL);
-
-	hv_cpuid_entries = kvm_get_supported_hv_cpuid();
-	test_hv_cpuid(hv_cpuid_entries, kvm_cpu_has(X86_FEATURE_VMX));
-	free((void *)hv_cpuid_entries);
+	test_hv_cpuid(NULL, kvm_cpu_has(X86_FEATURE_VMX));
 
 out:
 	kvm_vm_free(vm);
-- 
2.47.1.688.g23fc6f90ad-goog

[PATCH 5/5] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC

[Sean Christopherson]

Add testcases to x86's Hyper-V CPUID test to verify that KVM advertises
support for features that require an in-kernel local APIC appropriately,
i.e. that KVM hides support from the vCPU-scoped ioctl if the VM doesn't
have an in-kernel local APIC.

Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 90c44765d584..f76b6a95b530 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -43,6 +43,7 @@ static bool smt_possible(void)
 
 static void test_hv_cpuid(struct kvm_vcpu *vcpu, bool evmcs_expected)
 {
+	const bool has_irqchip = !vcpu || vcpu->vm->has_irqchip;
 	const struct kvm_cpuid2 *hv_cpuid_entries;
 	int i;
 	int nent_expected = 10;
@@ -85,12 +86,19 @@ static void test_hv_cpuid(struct kvm_vcpu *vcpu, bool evmcs_expected)
 				    entry->eax, evmcs_expected
 				);
 			break;
+		case 0x40000003:
+			TEST_ASSERT(has_irqchip || !(entry->edx & BIT(19)),
+				    "Synthetic Timers should require in-kernel APIC");
+			break;
 		case 0x40000004:
 			test_val = entry->eax & (1UL << 18);
 
 			TEST_ASSERT(!!test_val == !smt_possible(),
 				    "NoNonArchitecturalCoreSharing bit"
 				    " doesn't reflect SMT setting");
+
+			TEST_ASSERT(has_irqchip || !(entry->eax & BIT(10)),
+				    "Cluster IPI (i.e. SEND_IPI) should require in-kernel APIC");
 			break;
 		case 0x4000000A:
 			TEST_ASSERT(entry->eax & (1UL << 19),
@@ -139,9 +147,14 @@ int main(int argc, char *argv[])
 
 	TEST_REQUIRE(kvm_has_cap(KVM_CAP_HYPERV_CPUID));
 
-	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+	/* Test the vCPU ioctl without an in-kernel local APIC. */
+	vm = vm_create_barebones();
+	vcpu = __vm_vcpu_add(vm, 0);
+	test_hv_cpuid(vcpu, false);
+	kvm_vm_free(vm);
 
 	/* Test vCPU ioctl version */
+	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
 	test_hv_cpuid_e2big(vm, vcpu);
 	test_hv_cpuid(vcpu, false);
 
-- 
2.47.1.688.g23fc6f90ad-goog

Re: [PATCH 1/5] KVM: x86: Reject Hyper-V's SEND_IPI hypercalls if local APIC isn't in-kernel

[Sean Christopherson]

On Mon, Jan 13, 2025, Sean Christopherson wrote:
> Advertise support for Hyper-V's SEND_IPI and SEND_IPI_EX hypercalls if and
> only if the local API is emulated/virtualized by KVM, and explicitly reject
> said hypercalls if the local APIC is emulated in userspace, i.e. don't rely
> on userspace to opt-in to KVM_CAP_HYPERV_ENFORCE_CPUID.
> 
> Rejecting SEND_IPI and SEND_IPI_EX fixes a NULL-pointer dereference if
> Hyper-V enlightenments are exposed to the guest without an in-kernel local
> APIC:
> 
>   dump_stack+0xbe/0xfd
>   __kasan_report.cold+0x34/0x84
>   kasan_report+0x3a/0x50
>   __apic_accept_irq+0x3a/0x5c0
>   kvm_hv_send_ipi.isra.0+0x34e/0x820
>   kvm_hv_hypercall+0x8d9/0x9d0
>   kvm_emulate_hypercall+0x506/0x7e0
>   __vmx_handle_exit+0x283/0xb60
>   vmx_handle_exit+0x1d/0xd0
>   vcpu_enter_guest+0x16b0/0x24c0
>   vcpu_run+0xc0/0x550
>   kvm_arch_vcpu_ioctl_run+0x170/0x6d0
>   kvm_vcpu_ioctl+0x413/0xb20
>   __se_sys_ioctl+0x111/0x160
>   do_syscal1_64+0x30/0x40
>   entry_SYSCALL_64_after_hwframe+0x67/0xd1
> 
> Note, checking the sending vCPU is sufficient, as the per-VM irqchip_mode
> can't be modified after vCPUs are created, i.e. if one vCPU has an
> in-kernel local APIC, then all vCPUs have an in-kernel local APIC.
> 
> Reported-by: Dongjie Zou <zoudongjie@huawei.com>
> Fixes: 214ff83d4473 ("KVM: x86: hyperv: implement PV IPI send hypercalls")
> Fixes: 2bc39970e932 ("x86/kvm/hyper-v: Introduce KVM_GET_SUPPORTED_HV_CPUID")
> Cc: stable@vger.kernel

Argh, I missed the ".org".

Paolo, if you end up applying this instead of me, can you fixup the email?

Re: [PATCH 3/5] KVM: selftests: Explicitly free CPUID array at end of Hyper-V CPUID test

[Vitaly Kuznetsov]

Sean Christopherson <seanjc@google.com> writes:

> Explicitly free the array of CPUID entries at the end of the Hyper-V CPUID
> test, mainly in anticipation of moving management of the array into the
> main test helper.
>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> index 9a0fcc713350..09f9874d7705 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> @@ -164,6 +164,7 @@ int main(int argc, char *argv[])
>  
>  	hv_cpuid_entries = kvm_get_supported_hv_cpuid();
>  	test_hv_cpuid(hv_cpuid_entries, kvm_cpu_has(X86_FEATURE_VMX));
> +	free((void *)hv_cpuid_entries);

vcpu_get_supported_hv_cpuid() allocates memory for the resulting array
each time, however, kvm_get_supported_hv_cpuid() was designed after
what's now kvm_get_supported_cpuid() (afair) so it has an optimization
to ask KVM just once:

        static struct kvm_cpuid2 *cpuid;
        int kvm_fd;

        if (cpuid)
                return cpuid;

        cpuid = allocate_kvm_cpuid2(MAX_NR_CPUID_ENTRIES);
        kvm_fd = open_kvm_dev_path_or_exit();
	...

and it seems that if we free hv_cpuid_entries here, next time we call
kvm_get_supported_hv_cpuid() an already freed memory will be returned.
This doesn't matter in in this patch as we're about to quit anyway but
with the next one in the series it becomes problematic.

>  
>  out:
>  	kvm_vm_free(vm);

-- 
Vitaly

Re: [PATCH 4/5] KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper

[Vitaly Kuznetsov]

Sean Christopherson <seanjc@google.com> writes:

> Allocate, get, and free the CPUID array in the Hyper-V CPUID test in the
> test's core helper, instead of copy+pasting code at each call site.  In
> addition to deduplicating a small amount of code, restricting visibility
> of the array to a single invocation of the core test prevents "leaking" an
> array across test cases.  Passing in @vcpu to the helper will also allow
> pivoting on VM-scoped information without needing to pass more booleans,
> e.g. to conditionally assert on features that require an in-kernel APIC.
>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  .../selftests/kvm/x86_64/hyperv_cpuid.c       | 25 ++++++++-----------
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> index 09f9874d7705..90c44765d584 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> @@ -41,13 +41,18 @@ static bool smt_possible(void)
>  	return res;
>  }
>  
> -static void test_hv_cpuid(const struct kvm_cpuid2 *hv_cpuid_entries,
> -			  bool evmcs_expected)
> +static void test_hv_cpuid(struct kvm_vcpu *vcpu, bool evmcs_expected)
>  {
> +	const struct kvm_cpuid2 *hv_cpuid_entries;
>  	int i;
>  	int nent_expected = 10;
>  	u32 test_val;
>  
> +	if (vcpu)
> +		hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu);
> +	else
> +		hv_cpuid_entries = kvm_get_supported_hv_cpuid();
> +
>  	TEST_ASSERT(hv_cpuid_entries->nent == nent_expected,
>  		    "KVM_GET_SUPPORTED_HV_CPUID should return %d entries"
>  		    " (returned %d)",
> @@ -109,6 +114,7 @@ static void test_hv_cpuid(const struct kvm_cpuid2 *hv_cpuid_entries,
>  		 *	entry->edx);
>  		 */
>  	}
> +	free((void *)hv_cpuid_entries);

(see my comment on "[PATCH 3/5] KVM: selftests: Explicitly free CPUID
array at end of Hyper-V CPUID test"): 

vcpu_get_supported_hv_cpuid() allocates memory for the resulting array
each time, however, kvm_get_supported_hv_cpuid() doesn't so freeing
hv_cpuid_entries here will result in returning already freed memory next
time kvm_get_supported_hv_cpuid() path is taken.

>  }
>  
>  static void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> @@ -129,7 +135,6 @@ static void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
>  int main(int argc, char *argv[])
>  {
>  	struct kvm_vm *vm;
> -	const struct kvm_cpuid2 *hv_cpuid_entries;
>  	struct kvm_vcpu *vcpu;
>  
>  	TEST_REQUIRE(kvm_has_cap(KVM_CAP_HYPERV_CPUID));
> @@ -138,10 +143,7 @@ int main(int argc, char *argv[])
>  
>  	/* Test vCPU ioctl version */
>  	test_hv_cpuid_e2big(vm, vcpu);
> -
> -	hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu);
> -	test_hv_cpuid(hv_cpuid_entries, false);
> -	free((void *)hv_cpuid_entries);
> +	test_hv_cpuid(vcpu, false);
>  
>  	if (!kvm_cpu_has(X86_FEATURE_VMX) ||
>  	    !kvm_has_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
> @@ -149,9 +151,7 @@ int main(int argc, char *argv[])
>  		goto do_sys;
>  	}
>  	vcpu_enable_evmcs(vcpu);
> -	hv_cpuid_entries = vcpu_get_supported_hv_cpuid(vcpu);
> -	test_hv_cpuid(hv_cpuid_entries, true);
> -	free((void *)hv_cpuid_entries);
> +	test_hv_cpuid(vcpu, true);
>  
>  do_sys:
>  	/* Test system ioctl version */
> @@ -161,10 +161,7 @@ int main(int argc, char *argv[])
>  	}
>  
>  	test_hv_cpuid_e2big(vm, NULL);
> -
> -	hv_cpuid_entries = kvm_get_supported_hv_cpuid();
> -	test_hv_cpuid(hv_cpuid_entries, kvm_cpu_has(X86_FEATURE_VMX));
> -	free((void *)hv_cpuid_entries);
> +	test_hv_cpuid(NULL, kvm_cpu_has(X86_FEATURE_VMX));
>  
>  out:
>  	kvm_vm_free(vm);

-- 
Vitaly

Re: [PATCH 2/5] KVM: selftests: Mark test_hv_cpuid_e2big() static in Hyper-V CPUID test

[Vitaly Kuznetsov]

Sean Christopherson <seanjc@google.com> writes:

> Make the Hyper-V CPUID test's local helper test_hv_cpuid_e2big() static,
> it's not used outside of the test (and isn't intended to be).
>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> index 4f5881d4ef66..9a0fcc713350 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> @@ -111,7 +111,7 @@ static void test_hv_cpuid(const struct kvm_cpuid2 *hv_cpuid_entries,
>  	}
>  }
>  
> -void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> +static void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
>  {
>  	static struct kvm_cpuid2 cpuid = {.nent = 0};
>  	int ret;

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

-- 
Vitaly

Re: [PATCH 1/5] KVM: x86: Reject Hyper-V's SEND_IPI hypercalls if local APIC isn't in-kernel

[Vitaly Kuznetsov]

Sean Christopherson <seanjc@google.com> writes:

> Advertise support for Hyper-V's SEND_IPI and SEND_IPI_EX hypercalls if and
> only if the local API is emulated/virtualized by KVM, and explicitly reject
> said hypercalls if the local APIC is emulated in userspace, i.e. don't rely
> on userspace to opt-in to KVM_CAP_HYPERV_ENFORCE_CPUID.
>
> Rejecting SEND_IPI and SEND_IPI_EX fixes a NULL-pointer dereference if
> Hyper-V enlightenments are exposed to the guest without an in-kernel local
> APIC:
>
>   dump_stack+0xbe/0xfd
>   __kasan_report.cold+0x34/0x84
>   kasan_report+0x3a/0x50
>   __apic_accept_irq+0x3a/0x5c0
>   kvm_hv_send_ipi.isra.0+0x34e/0x820
>   kvm_hv_hypercall+0x8d9/0x9d0
>   kvm_emulate_hypercall+0x506/0x7e0
>   __vmx_handle_exit+0x283/0xb60
>   vmx_handle_exit+0x1d/0xd0
>   vcpu_enter_guest+0x16b0/0x24c0
>   vcpu_run+0xc0/0x550
>   kvm_arch_vcpu_ioctl_run+0x170/0x6d0
>   kvm_vcpu_ioctl+0x413/0xb20
>   __se_sys_ioctl+0x111/0x160
>   do_syscal1_64+0x30/0x40
>   entry_SYSCALL_64_after_hwframe+0x67/0xd1
>
> Note, checking the sending vCPU is sufficient, as the per-VM irqchip_mode
> can't be modified after vCPUs are created, i.e. if one vCPU has an
> in-kernel local APIC, then all vCPUs have an in-kernel local APIC.
>
> Reported-by: Dongjie Zou <zoudongjie@huawei.com>
> Fixes: 214ff83d4473 ("KVM: x86: hyperv: implement PV IPI send hypercalls")
> Fixes: 2bc39970e932 ("x86/kvm/hyper-v: Introduce KVM_GET_SUPPORTED_HV_CPUID")
> Cc: stable@vger.kernel

.org, as mentioned already

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/hyperv.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 4f0a94346d00..44c88537448c 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -2226,6 +2226,9 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  	u32 vector;
>  	bool all_cpus;
>  
> +	if (!lapic_in_kernel(vcpu))
> +		return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +
>  	if (hc->code == HVCALL_SEND_IPI) {
>  		if (!hc->fast) {
>  			if (unlikely(kvm_read_guest(kvm, hc->ingpa, &send_ipi,
> @@ -2852,7 +2855,8 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>  			ent->eax |= HV_X64_REMOTE_TLB_FLUSH_RECOMMENDED;
>  			ent->eax |= HV_X64_APIC_ACCESS_RECOMMENDED;
>  			ent->eax |= HV_X64_RELAXED_TIMING_RECOMMENDED;
> -			ent->eax |= HV_X64_CLUSTER_IPI_RECOMMENDED;
> +			if (!vcpu || lapic_in_kernel(vcpu))
> +				ent->eax |= HV_X64_CLUSTER_IPI_RECOMMENDED;
>  			ent->eax |= HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED;
>  			if (evmcs_ver)
>  				ent->eax |= HV_X64_ENLIGHTENED_VMCS_RECOMMENDED;

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

-- 
Vitaly

Re: [PATCH 3/5] KVM: selftests: Explicitly free CPUID array at end of Hyper-V CPUID test

[Sean Christopherson]

On Fri, Jan 17, 2025, Vitaly Kuznetsov wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
> > Explicitly free the array of CPUID entries at the end of the Hyper-V CPUID
> > test, mainly in anticipation of moving management of the array into the
> > main test helper.
> >
> > Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> > index 9a0fcc713350..09f9874d7705 100644
> > --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> > @@ -164,6 +164,7 @@ int main(int argc, char *argv[])
> >  
> >  	hv_cpuid_entries = kvm_get_supported_hv_cpuid();
> >  	test_hv_cpuid(hv_cpuid_entries, kvm_cpu_has(X86_FEATURE_VMX));
> > +	free((void *)hv_cpuid_entries);
> 
> vcpu_get_supported_hv_cpuid() allocates memory for the resulting array
> each time, however, kvm_get_supported_hv_cpuid() was designed after
> what's now kvm_get_supported_cpuid() (afair) so it has an optimization
> to ask KVM just once:
> 
>         static struct kvm_cpuid2 *cpuid;
>         int kvm_fd;
> 
>         if (cpuid)
>                 return cpuid;
> 
>         cpuid = allocate_kvm_cpuid2(MAX_NR_CPUID_ENTRIES);
>         kvm_fd = open_kvm_dev_path_or_exit();
> 	...
> 
> and it seems that if we free hv_cpuid_entries here, next time we call
> kvm_get_supported_hv_cpuid() an already freed memory will be returned.
> This doesn't matter in in this patch as we're about to quit anyway but
> with the next one in the series it becomes problematic.

Ow.  I totally missed that.  I'll drop this patch, and then adjust the next one
to do:

	/*
	 * Note, the CPUID array returned by the system-scoped helper is a one-
	 * time allocation, i.e. must not be freed.
	 */
	if (vcpu)
		free((void *)hv_cpuid_entries);


I'll post a v2 once I've actually tested.

Thanks!