kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase
@ 2025-01-18  0:34 Sean Christopherson
  2025-01-18  0:34 ` [PATCH v2 1/4] KVM: x86: Reject Hyper-V's SEND_IPI hypercalls if local APIC isn't in-kernel Sean Christopherson
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Sean Christopherson @ 2025-01-18  0:34 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, Dongjie Zou

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.

v2
 - Fix the stable@ email.  Hilariously, I was _this_ close to sending this
   with stable@vger.kernel@kernel.org instead of stable@vger.kernel.org,
   *after* I wrote this exact blurb about fat-fingering the email a second
   time.  Thankfully, git send-email told me I was being stupid :-)
 - Don't free the system-scoped CPUID entries object. [Vitaly]
 - Collect reviews. [Vitaly] 

v1: https://lore.kernel.org/all/20250113222740.1481934-1-seanjc@google.com

Sean Christopherson (4):
  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: 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       | 47 +++++++++++++------
 2 files changed, 37 insertions(+), 16 deletions(-)


base-commit: a5546c2f0dc4f84727a4bb8a91633917929735f5
-- 
2.48.0.rc2.279.g1de40edade-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/4] KVM: x86: Reject Hyper-V's SEND_IPI hypercalls if local APIC isn't in-kernel
  2025-01-18  0:34 [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase Sean Christopherson
@ 2025-01-18  0:34 ` Sean Christopherson
  2025-01-18  0:34 ` [PATCH v2 2/4] KVM: selftests: Mark test_hv_cpuid_e2big() static in Hyper-V CPUID test Sean Christopherson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2025-01-18  0:34 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, Dongjie Zou

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
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
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.48.0.rc2.279.g1de40edade-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/4] KVM: selftests: Mark test_hv_cpuid_e2big() static in Hyper-V CPUID test
  2025-01-18  0:34 [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase Sean Christopherson
  2025-01-18  0:34 ` [PATCH v2 1/4] KVM: x86: Reject Hyper-V's SEND_IPI hypercalls if local APIC isn't in-kernel Sean Christopherson
@ 2025-01-18  0:34 ` Sean Christopherson
  2025-01-18  0:34 ` [PATCH v2 3/4] KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper Sean Christopherson
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2025-01-18  0:34 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, Dongjie Zou

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).

Reviewed-by: 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.48.0.rc2.279.g1de40edade-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/4] KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper
  2025-01-18  0:34 [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase Sean Christopherson
  2025-01-18  0:34 ` [PATCH v2 1/4] KVM: x86: Reject Hyper-V's SEND_IPI hypercalls if local APIC isn't in-kernel Sean Christopherson
  2025-01-18  0:34 ` [PATCH v2 2/4] KVM: selftests: Mark test_hv_cpuid_e2big() static in Hyper-V CPUID test Sean Christopherson
@ 2025-01-18  0:34 ` Sean Christopherson
  2025-01-20 14:20   ` Vitaly Kuznetsov
  2025-01-18  0:34 ` [PATCH v2 4/4] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC Sean Christopherson
  2025-02-15  0:50 ` [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase Sean Christopherson
  4 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2025-01-18  0:34 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, Dongjie Zou

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.

To avoid use-after-free bugs due to overzealous and careless developers,
opportunstically add a comment to explain that the system-scoped helper
caches the Hyper-V CPUID entries, i.e. that the caller is not responsible
for freeing the memory.

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

diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 9a0fcc713350..3188749ec6e1 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,13 @@ static void test_hv_cpuid(const struct kvm_cpuid2 *hv_cpuid_entries,
 		 *	entry->edx);
 		 */
 	}
+
+	/*
+	 * 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);
 }
 
 static void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
@@ -129,7 +141,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 +149,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 +157,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,9 +167,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));
+	test_hv_cpuid(NULL, kvm_cpu_has(X86_FEATURE_VMX));
 
 out:
 	kvm_vm_free(vm);
-- 
2.48.0.rc2.279.g1de40edade-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/4] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC
  2025-01-18  0:34 [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase Sean Christopherson
                   ` (2 preceding siblings ...)
  2025-01-18  0:34 ` [PATCH v2 3/4] KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper Sean Christopherson
@ 2025-01-18  0:34 ` Sean Christopherson
  2025-01-20 14:20   ` Vitaly Kuznetsov
  2025-02-15  0:50 ` [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase Sean Christopherson
  4 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2025-01-18  0:34 UTC (permalink / raw)
  To: Vitaly Kuznetsov, Sean Christopherson, Paolo Bonzini
  Cc: kvm, linux-kernel, Dongjie Zou

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 3188749ec6e1..8f26130dc30d 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),
@@ -145,9 +153,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.48.0.rc2.279.g1de40edade-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 4/4] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC
  2025-01-18  0:34 ` [PATCH v2 4/4] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC Sean Christopherson
@ 2025-01-20 14:20   ` Vitaly Kuznetsov
  2025-01-21 16:00     ` Sean Christopherson
  0 siblings, 1 reply; 10+ messages in thread
From: Vitaly Kuznetsov @ 2025-01-20 14:20 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Dongjie Zou

Sean Christopherson <seanjc@google.com> writes:

> 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 3188749ec6e1..8f26130dc30d 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");

Nitpick: BIT(19) of CPUID.0x40000003(EDX) advertises 'direct' mode
for Synthetic timers and that's what we have paired with
lapic_in_kernel() check. Thus, we may want to be a bit more specific and
say

"Direct Synthetic timers should require in-kernel APIC"
(personally, I'd prefer "Synthetic timers in 'direct' mode" name but
that's not how TLFS calls them)

or something similar. 

(feel free to address this small rant of mine upon commit or just ignore)

> +			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),
> @@ -145,9 +153,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);

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

-- 
Vitaly


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/4] KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper
  2025-01-18  0:34 ` [PATCH v2 3/4] KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper Sean Christopherson
@ 2025-01-20 14:20   ` Vitaly Kuznetsov
  0 siblings, 0 replies; 10+ messages in thread
From: Vitaly Kuznetsov @ 2025-01-20 14:20 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Dongjie Zou

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.
>
> To avoid use-after-free bugs due to overzealous and careless developers,
> opportunstically add a comment to explain that the system-scoped helper
> caches the Hyper-V CPUID entries, i.e. that the caller is not responsible
> for freeing the memory.
>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  .../selftests/kvm/x86_64/hyperv_cpuid.c       | 30 +++++++++++--------
>  1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> index 9a0fcc713350..3188749ec6e1 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,13 @@ static void test_hv_cpuid(const struct kvm_cpuid2 *hv_cpuid_entries,
>  		 *	entry->edx);
>  		 */
>  	}
> +
> +	/*
> +	 * 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);
>  }
>  
>  static void test_hv_cpuid_e2big(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> @@ -129,7 +141,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 +149,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 +157,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,9 +167,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));
> +	test_hv_cpuid(NULL, kvm_cpu_has(X86_FEATURE_VMX));
>  
>  out:
>  	kvm_vm_free(vm);

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

-- 
Vitaly


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 4/4] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC
  2025-01-20 14:20   ` Vitaly Kuznetsov
@ 2025-01-21 16:00     ` Sean Christopherson
  2025-01-21 16:29       ` Vitaly Kuznetsov
  0 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2025-01-21 16:00 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: Paolo Bonzini, kvm, linux-kernel, Dongjie Zou

On Mon, Jan 20, 2025, Vitaly Kuznetsov wrote:
> > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> > index 3188749ec6e1..8f26130dc30d 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");
> 
> Nitpick: BIT(19) of CPUID.0x40000003(EDX) advertises 'direct' mode
> for Synthetic timers and that's what we have paired with
> lapic_in_kernel() check. Thus, we may want to be a bit more specific and
> say
> 
> "Direct Synthetic timers should require in-kernel APIC"
> (personally, I'd prefer "Synthetic timers in 'direct' mode" name but
> that's not how TLFS calls them)

What about adding quotes to try and communicate that it's a property of Syntehtic
Timers?  E.g.

 "\"Direct\" Synthetic Timers should require in-kernel APIC");

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 4/4] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC
  2025-01-21 16:00     ` Sean Christopherson
@ 2025-01-21 16:29       ` Vitaly Kuznetsov
  0 siblings, 0 replies; 10+ messages in thread
From: Vitaly Kuznetsov @ 2025-01-21 16:29 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, linux-kernel, Dongjie Zou

Sean Christopherson <seanjc@google.com> writes:

> On Mon, Jan 20, 2025, Vitaly Kuznetsov wrote:
>> > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
>> > index 3188749ec6e1..8f26130dc30d 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");
>> 
>> Nitpick: BIT(19) of CPUID.0x40000003(EDX) advertises 'direct' mode
>> for Synthetic timers and that's what we have paired with
>> lapic_in_kernel() check. Thus, we may want to be a bit more specific and
>> say
>> 
>> "Direct Synthetic timers should require in-kernel APIC"
>> (personally, I'd prefer "Synthetic timers in 'direct' mode" name but
>> that's not how TLFS calls them)
>
> What about adding quotes to try and communicate that it's a property of Syntehtic
> Timers?  E.g.
>
>  "\"Direct\" Synthetic Timers should require in-kernel APIC");

Sounds good to me)

-- 
Vitaly


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase
  2025-01-18  0:34 [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase Sean Christopherson
                   ` (3 preceding siblings ...)
  2025-01-18  0:34 ` [PATCH v2 4/4] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC Sean Christopherson
@ 2025-02-15  0:50 ` Sean Christopherson
  4 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2025-02-15  0:50 UTC (permalink / raw)
  To: Sean Christopherson, Vitaly Kuznetsov, Paolo Bonzini
  Cc: kvm, linux-kernel, Dongjie Zou

On Fri, 17 Jan 2025 16:34:50 -0800, Sean Christopherson wrote:
> 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.
> 
> v2
>  - Fix the stable@ email.  Hilariously, I was _this_ close to sending this
>    with stable@vger.kernel@kernel.org instead of stable@vger.kernel.org,
>    *after* I wrote this exact blurb about fat-fingering the email a second
>    time.  Thankfully, git send-email told me I was being stupid :-)
>  - Don't free the system-scoped CPUID entries object. [Vitaly]
>  - Collect reviews. [Vitaly]
> 
> [...]

Applied to kvm-x86 fixes, thanks!

[1/4] KVM: x86: Reject Hyper-V's SEND_IPI hypercalls if local APIC isn't in-kernel
      https://github.com/kvm-x86/linux/commit/a8de7f100bb5
[2/4] KVM: selftests: Mark test_hv_cpuid_e2big() static in Hyper-V CPUID test
      https://github.com/kvm-x86/linux/commit/0b6db0dc43ee
[3/4] KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper
      https://github.com/kvm-x86/linux/commit/cd5a0c2f0fae
[4/4] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC
      https://github.com/kvm-x86/linux/commit/e36454461c5e

--
https://github.com/kvm-x86/linux/tree/next

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-02-15  0:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-18  0:34 [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase Sean Christopherson
2025-01-18  0:34 ` [PATCH v2 1/4] KVM: x86: Reject Hyper-V's SEND_IPI hypercalls if local APIC isn't in-kernel Sean Christopherson
2025-01-18  0:34 ` [PATCH v2 2/4] KVM: selftests: Mark test_hv_cpuid_e2big() static in Hyper-V CPUID test Sean Christopherson
2025-01-18  0:34 ` [PATCH v2 3/4] KVM: selftests: Manage CPUID array in Hyper-V CPUID test's core helper Sean Christopherson
2025-01-20 14:20   ` Vitaly Kuznetsov
2025-01-18  0:34 ` [PATCH v2 4/4] KVM: selftests: Add CPUID tests for Hyper-V features that need in-kernel APIC Sean Christopherson
2025-01-20 14:20   ` Vitaly Kuznetsov
2025-01-21 16:00     ` Sean Christopherson
2025-01-21 16:29       ` Vitaly Kuznetsov
2025-02-15  0:50 ` [PATCH v2 0/4] KVM: x86: Hyper-V SEND_IPI fix and partial testcase Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).