As there is no interception on AMD on the end of an NMI handler but only
on its iret, we are forced to step out by setting TF in rflags. This can
collide with host or guest using single-step mode, and it may leak the
flags onto the guest stack if IRET triggers some exception.
This patch addresses the TF leakage by trapping all exceptions that may
show up on IRET execution and cleaning up the state again before
reinjecting the exception. Collisions with concurrent users are avoided
by carefully checking for their presence and forwarding #DB exceptions
whenever required.
Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
---
arch/x86/include/asm/kvm_host.h | 3 +
arch/x86/kvm/svm.c | 112 +++++++++++++++++++++++++++++---------
2 files changed, 88 insertions(+), 27 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f9a2f66..cbae274 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -62,6 +62,7 @@
#define GP_VECTOR 13
#define PF_VECTOR 14
#define MF_VECTOR 16
+#define AC_VECTOR 17
#define MC_VECTOR 18
#define SELECTOR_TI_MASK (1 << 2)
@@ -131,8 +132,10 @@ enum {
#define KVM_NR_DB_REGS 4
+#define DR6_BP_MASK 0x0000000f
#define DR6_BD (1 << 13)
#define DR6_BS (1 << 14)
+#define DR6_BT (1 << 15)
#define DR6_FIXED_1 0xffff0ff0
#define DR6_VOLATILE 0x0000e00f
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index f63f1db..672f394 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -109,7 +109,8 @@ struct vcpu_svm {
struct nested_state nested;
- bool nmi_singlestep;
+ bool iret_singlestep;
+ u64 iret_rflags;
bool int3_injected;
};
@@ -1084,15 +1085,21 @@ static void svm_set_segment(struct kvm_vcpu *vcpu,
}
+#define IRET_EXCEPTIONS (1 << NP_VECTOR) | (1 << SS_VECTOR) | \
+ (1 << GP_VECTOR) | (1 << PF_VECTOR) | (1 << AC_VECTOR)
+
static void update_db_intercept(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
svm->vmcb->control.intercept_exceptions &=
- ~((1 << DB_VECTOR) | (1 << BP_VECTOR));
+ ~((1 << DB_VECTOR) | (1 << BP_VECTOR) | IRET_EXCEPTIONS);
+ if (!npt_enabled)
+ svm->vmcb->control.intercept_exceptions |= 1 << PF_VECTOR;
- if (svm->nmi_singlestep)
- svm->vmcb->control.intercept_exceptions |= (1 << DB_VECTOR);
+ if (svm->iret_singlestep)
+ svm->vmcb->control.intercept_exceptions |=
+ (1 << DB_VECTOR) | IRET_EXCEPTIONS;
if (vcpu->guest_debug & KVM_GUESTDBG_ENABLE) {
if (vcpu->guest_debug &
@@ -1210,11 +1217,45 @@ static int svm_set_dr(struct kvm_vcpu *vcpu, int dr, unsigned long value)
return EMULATE_DONE;
}
+static void reset_iret_singlestep(struct vcpu_svm * svm)
+{
+ svm->iret_singlestep = false;
+ svm->vmcb->save.rflags = svm->iret_rflags;
+ update_db_intercept(&svm->vcpu);
+
+ /* We did not yet leave the NMI handler. */
+ svm->vcpu.arch.hflags |= HF_NMI_MASK;
+}
+
+static int fault_on_iret(struct vcpu_svm *svm)
+{
+ u32 exit_code = svm->vmcb->control.exit_code;
+
+ BUG_ON(!svm->iret_singlestep);
+
+ reset_iret_singlestep(svm);
+
+ /*
+ * Requeue the exception IRET triggered and let the guest handle it.
+ * Note that all of them come with an error code.
+ */
+ kvm_queue_exception_e(&svm->vcpu, exit_code - SVM_EXIT_EXCP_BASE,
+ svm->vmcb->control.exit_int_info_err);
+ return 1;
+}
+
static int pf_interception(struct vcpu_svm *svm)
{
u64 fault_address;
u32 error_code;
+ if (svm->iret_singlestep) {
+ if (npt_enabled)
+ return fault_on_iret(svm);
+
+ reset_iret_singlestep(svm);
+ }
+
fault_address = svm->vmcb->control.exit_info_2;
error_code = svm->vmcb->control.exit_info_1;
@@ -1227,32 +1268,43 @@ static int pf_interception(struct vcpu_svm *svm)
static int db_interception(struct vcpu_svm *svm)
{
struct kvm_run *kvm_run = svm->vcpu.run;
+ struct vmcb_save_area *save = &svm->vmcb->save;
- if (!(svm->vcpu.guest_debug &
- (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)) &&
- !svm->nmi_singlestep) {
- kvm_queue_exception(&svm->vcpu, DB_VECTOR);
- return 1;
- }
+ if (svm->iret_singlestep) {
+ if (!(save->dr6 & DR6_BS)) {
+ /* Not yet the step we are waiting for. */
+ reset_iret_singlestep(svm);
+ goto check_guest_debug;
+ }
- if (svm->nmi_singlestep) {
- svm->nmi_singlestep = false;
- if (!(svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP))
- svm->vmcb->save.rflags &=
- ~(X86_EFLAGS_TF | X86_EFLAGS_RF);
+ svm->iret_singlestep = false;
update_db_intercept(&svm->vcpu);
+
+ if (svm->vcpu.guest_debug & KVM_GUESTDBG_SINGLESTEP)
+ goto debugger_exit;
+
+ /*
+ * Check if there is some other reason in DR6 or if the guest
+ * is in single-step mode as well. If not, we are done.
+ */
+ if (!(save->dr6 & (DR6_BP_MASK | DR6_BD | DR6_BT)) &&
+ (save->rflags & (X86_EFLAGS_TF | X86_EFLAGS_RF)) !=
+ X86_EFLAGS_TF)
+ return 1;
}
- if (svm->vcpu.guest_debug &
- (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP)){
- kvm_run->exit_reason = KVM_EXIT_DEBUG;
- kvm_run->debug.arch.pc =
- svm->vmcb->save.cs.base + svm->vmcb->save.rip;
- kvm_run->debug.arch.exception = DB_VECTOR;
- return 0;
+check_guest_debug:
+ if (!(svm->vcpu.guest_debug &
+ (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP))) {
+ kvm_queue_exception(&svm->vcpu, DB_VECTOR);
+ return 1;
}
- return 1;
+debugger_exit:
+ kvm_run->exit_reason = KVM_EXIT_DEBUG;
+ kvm_run->debug.arch.pc = svm->vmcb->save.cs.base + svm->vmcb->save.rip;
+ kvm_run->debug.arch.exception = DB_VECTOR;
+ return 0;
}
static int bp_interception(struct vcpu_svm *svm)
@@ -2367,6 +2419,10 @@ static int (*svm_exit_handlers[])(struct vcpu_svm *svm) = {
[SVM_EXIT_EXCP_BASE + PF_VECTOR] = pf_interception,
[SVM_EXIT_EXCP_BASE + NM_VECTOR] = nm_interception,
[SVM_EXIT_EXCP_BASE + MC_VECTOR] = mc_interception,
+ [SVM_EXIT_EXCP_BASE + NP_VECTOR] = fault_on_iret,
+ [SVM_EXIT_EXCP_BASE + SS_VECTOR] = fault_on_iret,
+ [SVM_EXIT_EXCP_BASE + GP_VECTOR] = fault_on_iret,
+ [SVM_EXIT_EXCP_BASE + AC_VECTOR] = fault_on_iret,
[SVM_EXIT_INTR] = intr_interception,
[SVM_EXIT_NMI] = nmi_interception,
[SVM_EXIT_SMI] = nop_on_interception,
@@ -2598,10 +2654,12 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
== HF_NMI_MASK)
return; /* IRET will cause a vm exit */
- /* Something prevents NMI from been injected. Single step over
- possible problem (IRET or exception injection or interrupt
- shadow) */
- svm->nmi_singlestep = true;
+ /*
+ * Step over IRET. We will try to inject again after returning from
+ * the NMI handler.
+ */
+ svm->iret_singlestep = true;
+ svm->iret_rflags = svm->vmcb->save.rflags;
svm->vmcb->save.rflags |= (X86_EFLAGS_TF | X86_EFLAGS_RF);
update_db_intercept(vcpu);
}
--
1.6.0.2
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
