Skip to content

Commit 54bcccc

Browse files
osandovgregkh
authored andcommitted
KVM: SVM: Don't skip unrelated instruction if INT3/INTO is replaced
commit 4da3768 upstream. When re-injecting a soft interrupt from an INT3, INT0, or (select) INTn instruction, discard the exception and retry the instruction if the code stream is changed (e.g. by a different vCPU) between when the CPU executes the instruction and when KVM decodes the instruction to get the next RIP. As effectively predicted by commit 6ef88d6 ("KVM: SVM: Re-inject INT3/INTO instead of retrying the instruction"), failure to verify that the correct INTn instruction was decoded can effectively clobber guest state due to decoding the wrong instruction and thus specifying the wrong next RIP. The bug most often manifests as "Oops: int3" panics on static branch checks in Linux guests. Enabling or disabling a static branch in Linux uses the kernel's "text poke" code patching mechanism. To modify code while other CPUs may be executing that code, Linux (temporarily) replaces the first byte of the original instruction with an int3 (opcode 0xcc), then patches in the new code stream except for the first byte, and finally replaces the int3 with the first byte of the new code stream. If a CPU hits the int3, i.e. executes the code while it's being modified, then the guest kernel must look up the RIP to determine how to handle the #BP, e.g. by emulating the new instruction. If the RIP is incorrect, then this lookup fails and the guest kernel panics. The bug reproduces almost instantly by hacking the guest kernel to repeatedly check a static branch[1] while running a drgn script[2] on the host to constantly swap out the memory containing the guest's TSS. [1]: https://gist.github.com/osandov/44d17c51c28c0ac998ea0334edf90b5a [2]: https://gist.github.com/osandov/10e45e45afa29b11e0c7209247afc00b Fixes: 6ef88d6 ("KVM: SVM: Re-inject INT3/INTO instead of retrying the instruction") Cc: stable@vger.kernel.org Co-developed-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Omar Sandoval <osandov@fb.com> Link: https://patch.msgid.link/1cc6dcdf36e3add7ee7c8d90ad58414eeb6c3d34.1762278762.git.osandov@fb.com Signed-off-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent d948c53 commit 54bcccc

3 files changed

Lines changed: 43 additions & 11 deletions

File tree

arch/x86/include/asm/kvm_host.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2123,6 +2123,11 @@ u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu);
21232123
* the gfn, i.e. retrying the instruction will hit a
21242124
* !PRESENT fault, which results in a new shadow page
21252125
* and sends KVM back to square one.
2126+
*
2127+
* EMULTYPE_SKIP_SOFT_INT - Set in combination with EMULTYPE_SKIP to only skip
2128+
* an instruction if it could generate a given software
2129+
* interrupt, which must be encoded via
2130+
* EMULTYPE_SET_SOFT_INT_VECTOR().
21262131
*/
21272132
#define EMULTYPE_NO_DECODE (1 << 0)
21282133
#define EMULTYPE_TRAP_UD (1 << 1)
@@ -2133,6 +2138,10 @@ u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu);
21332138
#define EMULTYPE_PF (1 << 6)
21342139
#define EMULTYPE_COMPLETE_USER_EXIT (1 << 7)
21352140
#define EMULTYPE_WRITE_PF_TO_SP (1 << 8)
2141+
#define EMULTYPE_SKIP_SOFT_INT (1 << 9)
2142+
2143+
#define EMULTYPE_SET_SOFT_INT_VECTOR(v) ((u32)((v) & 0xff) << 16)
2144+
#define EMULTYPE_GET_SOFT_INT_VECTOR(e) (((e) >> 16) & 0xff)
21362145

21372146
static inline bool kvm_can_emulate_event_vectoring(int emul_type)
21382147
{

arch/x86/kvm/svm/svm.c

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ static void svm_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask)
280280
}
281281

282282
static int __svm_skip_emulated_instruction(struct kvm_vcpu *vcpu,
283+
int emul_type,
283284
bool commit_side_effects)
284285
{
285286
struct vcpu_svm *svm = to_svm(vcpu);
@@ -301,7 +302,7 @@ static int __svm_skip_emulated_instruction(struct kvm_vcpu *vcpu,
301302
if (unlikely(!commit_side_effects))
302303
old_rflags = svm->vmcb->save.rflags;
303304

304-
if (!kvm_emulate_instruction(vcpu, EMULTYPE_SKIP))
305+
if (!kvm_emulate_instruction(vcpu, emul_type))
305306
return 0;
306307

307308
if (unlikely(!commit_side_effects))
@@ -319,11 +320,13 @@ static int __svm_skip_emulated_instruction(struct kvm_vcpu *vcpu,
319320

320321
static int svm_skip_emulated_instruction(struct kvm_vcpu *vcpu)
321322
{
322-
return __svm_skip_emulated_instruction(vcpu, true);
323+
return __svm_skip_emulated_instruction(vcpu, EMULTYPE_SKIP, true);
323324
}
324325

325-
static int svm_update_soft_interrupt_rip(struct kvm_vcpu *vcpu)
326+
static int svm_update_soft_interrupt_rip(struct kvm_vcpu *vcpu, u8 vector)
326327
{
328+
const int emul_type = EMULTYPE_SKIP | EMULTYPE_SKIP_SOFT_INT |
329+
EMULTYPE_SET_SOFT_INT_VECTOR(vector);
327330
unsigned long rip, old_rip = kvm_rip_read(vcpu);
328331
struct vcpu_svm *svm = to_svm(vcpu);
329332

@@ -339,7 +342,7 @@ static int svm_update_soft_interrupt_rip(struct kvm_vcpu *vcpu)
339342
* in use, the skip must not commit any side effects such as clearing
340343
* the interrupt shadow or RFLAGS.RF.
341344
*/
342-
if (!__svm_skip_emulated_instruction(vcpu, !nrips))
345+
if (!__svm_skip_emulated_instruction(vcpu, emul_type, !nrips))
343346
return -EIO;
344347

345348
rip = kvm_rip_read(vcpu);
@@ -375,7 +378,7 @@ static void svm_inject_exception(struct kvm_vcpu *vcpu)
375378
kvm_deliver_exception_payload(vcpu, ex);
376379

377380
if (kvm_exception_is_soft(ex->vector) &&
378-
svm_update_soft_interrupt_rip(vcpu))
381+
svm_update_soft_interrupt_rip(vcpu, ex->vector))
379382
return;
380383

381384
svm->vmcb->control.event_inj = ex->vector
@@ -3662,24 +3665,23 @@ static bool svm_set_vnmi_pending(struct kvm_vcpu *vcpu)
36623665

36633666
static void svm_inject_irq(struct kvm_vcpu *vcpu, bool reinjected)
36643667
{
3668+
struct kvm_queued_interrupt *intr = &vcpu->arch.interrupt;
36653669
struct vcpu_svm *svm = to_svm(vcpu);
36663670
u32 type;
36673671

3668-
if (vcpu->arch.interrupt.soft) {
3669-
if (svm_update_soft_interrupt_rip(vcpu))
3672+
if (intr->soft) {
3673+
if (svm_update_soft_interrupt_rip(vcpu, intr->nr))
36703674
return;
36713675

36723676
type = SVM_EVTINJ_TYPE_SOFT;
36733677
} else {
36743678
type = SVM_EVTINJ_TYPE_INTR;
36753679
}
36763680

3677-
trace_kvm_inj_virq(vcpu->arch.interrupt.nr,
3678-
vcpu->arch.interrupt.soft, reinjected);
3681+
trace_kvm_inj_virq(intr->nr, intr->soft, reinjected);
36793682
++vcpu->stat.irq_injections;
36803683

3681-
svm->vmcb->control.event_inj = vcpu->arch.interrupt.nr |
3682-
SVM_EVTINJ_VALID | type;
3684+
svm->vmcb->control.event_inj = intr->nr | SVM_EVTINJ_VALID | type;
36833685
}
36843686

36853687
void svm_complete_interrupt_delivery(struct kvm_vcpu *vcpu, int delivery_mode,

arch/x86/kvm/x86.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9021,6 +9021,23 @@ static bool is_vmware_backdoor_opcode(struct x86_emulate_ctxt *ctxt)
90219021
return false;
90229022
}
90239023

9024+
static bool is_soft_int_instruction(struct x86_emulate_ctxt *ctxt,
9025+
int emulation_type)
9026+
{
9027+
u8 vector = EMULTYPE_GET_SOFT_INT_VECTOR(emulation_type);
9028+
9029+
switch (ctxt->b) {
9030+
case 0xcc:
9031+
return vector == BP_VECTOR;
9032+
case 0xcd:
9033+
return vector == ctxt->src.val;
9034+
case 0xce:
9035+
return vector == OF_VECTOR;
9036+
default:
9037+
return false;
9038+
}
9039+
}
9040+
90249041
/*
90259042
* Decode an instruction for emulation. The caller is responsible for handling
90269043
* code breakpoints. Note, manually detecting code breakpoints is unnecessary
@@ -9131,6 +9148,10 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
91319148
* injecting single-step #DBs.
91329149
*/
91339150
if (emulation_type & EMULTYPE_SKIP) {
9151+
if (emulation_type & EMULTYPE_SKIP_SOFT_INT &&
9152+
!is_soft_int_instruction(ctxt, emulation_type))
9153+
return 0;
9154+
91349155
if (ctxt->mode != X86EMUL_MODE_PROT64)
91359156
ctxt->eip = (u32)ctxt->_eip;
91369157
else

0 commit comments

Comments
 (0)