* [PATCH 1/2] KVM: selftests: Reload "good" vCPU state if vCPU hits shutdown
2023-08-17 23:34 [PATCH 0/2] KVM: selftests: Fix sync_regs race tests on AMD Sean Christopherson
@ 2023-08-17 23:34 ` Sean Christopherson
2023-08-17 23:34 ` [PATCH 2/2] KVM: selftests: Explicit set #UD when *potentially* injecting exception Sean Christopherson
2023-08-25 19:01 ` [PATCH 0/2] KVM: selftests: Fix sync_regs race tests on AMD Sean Christopherson
2 siblings, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2023-08-17 23:34 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Michal Luczaj
Reload known good vCPU state if the vCPU triple faults in any of the
race_sync_regs() subtests, e.g. if KVM successfully injects an exception
(the vCPU isn't configured to handle exceptions). On Intel, the VMCS
is preserved even after shutdown, but AMD's APM states that the VMCB is
undefined after a shutdown and so KVM synthesizes an INIT to sanitize
vCPU/VMCB state, e.g. to guard against running with a garbage VMCB.
The synthetic INIT results in the vCPU never exiting to userspace, as it
gets put into Real Mode at the reset vector, which is full of zeros (as is
GPA 0 and beyond), and so executes ADD for a very, very long time.
Fixes: 60c4063b4752 ("KVM: selftests: Extend x86's sync_regs_test to check for event vector races")
Cc: Michal Luczaj <mhal@rbox.co>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
.../testing/selftests/kvm/x86_64/sync_regs_test.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
index 93fac74ca0a7..21e99dae2ff2 100644
--- a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
@@ -152,6 +152,7 @@ static noinline void *race_sregs_cr4(void *arg)
static void race_sync_regs(void *racer)
{
const time_t TIMEOUT = 2; /* seconds, roughly */
+ struct kvm_x86_state *state;
struct kvm_translation tr;
struct kvm_vcpu *vcpu;
struct kvm_run *run;
@@ -166,6 +167,9 @@ static void race_sync_regs(void *racer)
vcpu_run(vcpu);
run->kvm_valid_regs = 0;
+ /* Save state *before* spawning the thread that mucks with vCPU state. */
+ state = vcpu_save_state(vcpu);
+
/*
* Selftests run 64-bit guests by default, both EFER.LME and CR4.PAE
* should already be set in guest state.
@@ -179,7 +183,14 @@ static void race_sync_regs(void *racer)
TEST_ASSERT_EQ(pthread_create(&thread, NULL, racer, (void *)run), 0);
for (t = time(NULL) + TIMEOUT; time(NULL) < t;) {
- __vcpu_run(vcpu);
+ /*
+ * Reload known good state if the vCPU triple faults, e.g. due
+ * to the unhandled #GPs being injected. VMX preserves state
+ * on shutdown, but SVM synthesizes an INIT as the VMCB state
+ * is architecturally undefined on triple fault.
+ */
+ if (!__vcpu_run(vcpu) && run->exit_reason == KVM_EXIT_SHUTDOWN)
+ vcpu_load_state(vcpu, state);
if (racer == race_sregs_cr4) {
tr = (struct kvm_translation) { .linear_address = 0 };
@@ -190,6 +201,7 @@ static void race_sync_regs(void *racer)
TEST_ASSERT_EQ(pthread_cancel(thread), 0);
TEST_ASSERT_EQ(pthread_join(thread, NULL), 0);
+ kvm_x86_state_cleanup(state);
kvm_vm_free(vm);
}
--
2.42.0.rc1.204.g551eb34607-goog
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/2] KVM: selftests: Explicit set #UD when *potentially* injecting exception
2023-08-17 23:34 [PATCH 0/2] KVM: selftests: Fix sync_regs race tests on AMD Sean Christopherson
2023-08-17 23:34 ` [PATCH 1/2] KVM: selftests: Reload "good" vCPU state if vCPU hits shutdown Sean Christopherson
@ 2023-08-17 23:34 ` Sean Christopherson
2023-08-25 19:01 ` [PATCH 0/2] KVM: selftests: Fix sync_regs race tests on AMD Sean Christopherson
2 siblings, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2023-08-17 23:34 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Michal Luczaj
Explicitly set the exception vector to #UD when potentially injecting an
exception in sync_regs_test's subtests that try to detect TOCTOU bugs
in KVM's handling of exceptions injected by userspace. A side effect of
the original KVM bug was that KVM would clear the vector, but relying on
KVM to clear the vector (i.e. make it #DE) makes it less likely that the
test would ever find *new* KVM bugs, e.g. because only the first iteration
would run with a legal vector to start.
Explicitly inject #UD for race_events_inj_pen() as well, e.g. so that it
doesn't inherit the illegal 255 vector from race_events_exc(), which
currently runs first.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/x86_64/sync_regs_test.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
index 21e99dae2ff2..00965ba33f73 100644
--- a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
@@ -91,6 +91,8 @@ static void *race_events_inj_pen(void *arg)
struct kvm_run *run = (struct kvm_run *)arg;
struct kvm_vcpu_events *events = &run->s.regs.events;
+ WRITE_ONCE(events->exception.nr, UD_VECTOR);
+
for (;;) {
WRITE_ONCE(run->kvm_dirty_regs, KVM_SYNC_X86_EVENTS);
WRITE_ONCE(events->flags, 0);
@@ -115,6 +117,7 @@ static void *race_events_exc(void *arg)
for (;;) {
WRITE_ONCE(run->kvm_dirty_regs, KVM_SYNC_X86_EVENTS);
WRITE_ONCE(events->flags, 0);
+ WRITE_ONCE(events->exception.nr, UD_VECTOR);
WRITE_ONCE(events->exception.pending, 1);
WRITE_ONCE(events->exception.nr, 255);
--
2.42.0.rc1.204.g551eb34607-goog
^ permalink raw reply [flat|nested] 4+ messages in thread