Skip to content

Commit f211b45

Browse files
committed
Merge tag 'kvm-x86-fixes-6.4' of https://github.com/kvm-x86/linux into HEAD
KVM x86 fixes for 6.4 - Fix a memslot lookup bug in the NX recovery thread that could theoretically let userspace bypass the NX hugepage mitigation - Fix a s/BLOCKING/PENDING bug in SVM's vNMI support - Account exit stats for fastpath VM-Exits that never leave the super tight run-loop - Fix an out-of-bounds bug in the optimized APIC map code, and add a regression test for the race.
2 parents 49661a5 + 47d2804 commit f211b45

File tree

6 files changed

+101
-4
lines changed

6 files changed

+101
-4
lines changed

arch/x86/kvm/lapic.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,23 @@ static int kvm_recalculate_phys_map(struct kvm_apic_map *new,
228228
u32 xapic_id = kvm_xapic_id(apic);
229229
u32 physical_id;
230230

231+
/*
232+
* For simplicity, KVM always allocates enough space for all possible
233+
* xAPIC IDs. Yell, but don't kill the VM, as KVM can continue on
234+
* without the optimized map.
235+
*/
236+
if (WARN_ON_ONCE(xapic_id > new->max_apic_id))
237+
return -EINVAL;
238+
239+
/*
240+
* Bail if a vCPU was added and/or enabled its APIC between allocating
241+
* the map and doing the actual calculations for the map. Note, KVM
242+
* hardcodes the x2APIC ID to vcpu_id, i.e. there's no TOCTOU bug if
243+
* the compiler decides to reload x2apic_id after this check.
244+
*/
245+
if (x2apic_id > new->max_apic_id)
246+
return -E2BIG;
247+
231248
/*
232249
* Deliberately truncate the vCPU ID when detecting a mismatched APIC
233250
* ID to avoid false positives if the vCPU ID, i.e. x2APIC ID, is a
@@ -253,8 +270,7 @@ static int kvm_recalculate_phys_map(struct kvm_apic_map *new,
253270
*/
254271
if (vcpu->kvm->arch.x2apic_format) {
255272
/* See also kvm_apic_match_physical_addr(). */
256-
if ((apic_x2apic_mode(apic) || x2apic_id > 0xff) &&
257-
x2apic_id <= new->max_apic_id)
273+
if (apic_x2apic_mode(apic) || x2apic_id > 0xff)
258274
new->phys_map[x2apic_id] = apic;
259275

260276
if (!apic_x2apic_mode(apic) && !new->phys_map[xapic_id])

arch/x86/kvm/mmu/mmu.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7091,7 +7091,10 @@ static void kvm_recover_nx_huge_pages(struct kvm *kvm)
70917091
*/
70927092
slot = NULL;
70937093
if (atomic_read(&kvm->nr_memslots_dirty_logging)) {
7094-
slot = gfn_to_memslot(kvm, sp->gfn);
7094+
struct kvm_memslots *slots;
7095+
7096+
slots = kvm_memslots_for_spte_role(kvm, sp->role);
7097+
slot = __gfn_to_memslot(slots, sp->gfn);
70957098
WARN_ON_ONCE(!slot);
70967099
}
70977100

arch/x86/kvm/svm/svm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3510,7 +3510,7 @@ static bool svm_is_vnmi_pending(struct kvm_vcpu *vcpu)
35103510
if (!is_vnmi_enabled(svm))
35113511
return false;
35123512

3513-
return !!(svm->vmcb->control.int_ctl & V_NMI_BLOCKING_MASK);
3513+
return !!(svm->vmcb->control.int_ctl & V_NMI_PENDING_MASK);
35143514
}
35153515

35163516
static bool svm_set_vnmi_pending(struct kvm_vcpu *vcpu)

arch/x86/kvm/x86.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10758,6 +10758,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
1075810758
exit_fastpath = EXIT_FASTPATH_EXIT_HANDLED;
1075910759
break;
1076010760
}
10761+
10762+
/* Note, VM-Exits that go down the "slow" path are accounted below. */
10763+
++vcpu->stat.exits;
1076110764
}
1076210765

1076310766
/*

tools/testing/selftests/kvm/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/sev_migrate_tests
116116
TEST_GEN_PROGS_x86_64 += x86_64/amx_test
117117
TEST_GEN_PROGS_x86_64 += x86_64/max_vcpuid_cap_test
118118
TEST_GEN_PROGS_x86_64 += x86_64/triple_fault_event_test
119+
TEST_GEN_PROGS_x86_64 += x86_64/recalc_apic_map_test
119120
TEST_GEN_PROGS_x86_64 += access_tracking_perf_test
120121
TEST_GEN_PROGS_x86_64 += demand_paging_test
121122
TEST_GEN_PROGS_x86_64 += dirty_log_test
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// SPDX-License-Identifier: GPL-2.0-only
2+
/*
3+
* Test edge cases and race conditions in kvm_recalculate_apic_map().
4+
*/
5+
6+
#include <sys/ioctl.h>
7+
#include <pthread.h>
8+
#include <time.h>
9+
10+
#include "processor.h"
11+
#include "test_util.h"
12+
#include "kvm_util.h"
13+
#include "apic.h"
14+
15+
#define TIMEOUT 5 /* seconds */
16+
17+
#define LAPIC_DISABLED 0
18+
#define LAPIC_X2APIC (MSR_IA32_APICBASE_ENABLE | X2APIC_ENABLE)
19+
#define MAX_XAPIC_ID 0xff
20+
21+
static void *race(void *arg)
22+
{
23+
struct kvm_lapic_state lapic = {};
24+
struct kvm_vcpu *vcpu = arg;
25+
26+
while (1) {
27+
/* Trigger kvm_recalculate_apic_map(). */
28+
vcpu_ioctl(vcpu, KVM_SET_LAPIC, &lapic);
29+
pthread_testcancel();
30+
}
31+
32+
return NULL;
33+
}
34+
35+
int main(void)
36+
{
37+
struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
38+
struct kvm_vcpu *vcpuN;
39+
struct kvm_vm *vm;
40+
pthread_t thread;
41+
time_t t;
42+
int i;
43+
44+
kvm_static_assert(KVM_MAX_VCPUS > MAX_XAPIC_ID);
45+
46+
/*
47+
* Create the max number of vCPUs supported by selftests so that KVM
48+
* has decent amount of work to do when recalculating the map, i.e. to
49+
* make the problematic window large enough to hit.
50+
*/
51+
vm = vm_create_with_vcpus(KVM_MAX_VCPUS, NULL, vcpus);
52+
53+
/*
54+
* Enable x2APIC on all vCPUs so that KVM doesn't bail from the recalc
55+
* due to vCPUs having aliased xAPIC IDs (truncated to 8 bits).
56+
*/
57+
for (i = 0; i < KVM_MAX_VCPUS; i++)
58+
vcpu_set_msr(vcpus[i], MSR_IA32_APICBASE, LAPIC_X2APIC);
59+
60+
ASSERT_EQ(pthread_create(&thread, NULL, race, vcpus[0]), 0);
61+
62+
vcpuN = vcpus[KVM_MAX_VCPUS - 1];
63+
for (t = time(NULL) + TIMEOUT; time(NULL) < t;) {
64+
vcpu_set_msr(vcpuN, MSR_IA32_APICBASE, LAPIC_X2APIC);
65+
vcpu_set_msr(vcpuN, MSR_IA32_APICBASE, LAPIC_DISABLED);
66+
}
67+
68+
ASSERT_EQ(pthread_cancel(thread), 0);
69+
ASSERT_EQ(pthread_join(thread, NULL), 0);
70+
71+
kvm_vm_free(vm);
72+
73+
return 0;
74+
}

0 commit comments

Comments
 (0)