Skip to content

Commit 33ab911

Browse files
committed
KVM: x86: fix emulation of "MOV SS, null selector"
This is CVE-2017-2583. On Intel this causes a failed vmentry because SS's type is neither 3 nor 7 (even though the manual says this check is only done for usable SS, and the dmesg splat says that SS is unusable!). On AMD it's worse: svm.c is confused and sets CPL to 0 in the vmcb. The fix fabricates a data segment descriptor when SS is set to a null selector, so that CPL and SS.DPL are set correctly in the VMCS/vmcb. Furthermore, only allow setting SS to a NULL selector if SS.RPL < 3; this in turn ensures CPL < 3 because RPL must be equal to CPL. Thanks to Andy Lutomirski and Willy Tarreau for help in analyzing the bug and deciphering the manuals. Reported-by: Xiaohan Zhang <[email protected]> Fixes: 79d5b4c Cc: [email protected] Signed-off-by: Paolo Bonzini <[email protected]>
1 parent 546d87e commit 33ab911

File tree

1 file changed

+38
-10
lines changed

1 file changed

+38
-10
lines changed

arch/x86/kvm/emulate.c

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,7 +1585,6 @@ static int write_segment_descriptor(struct x86_emulate_ctxt *ctxt,
15851585
&ctxt->exception);
15861586
}
15871587

1588-
/* Does not support long mode */
15891588
static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
15901589
u16 selector, int seg, u8 cpl,
15911590
enum x86_transfer_type transfer,
@@ -1622,20 +1621,34 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
16221621

16231622
rpl = selector & 3;
16241623

1625-
/* NULL selector is not valid for TR, CS and SS (except for long mode) */
1626-
if ((seg == VCPU_SREG_CS
1627-
|| (seg == VCPU_SREG_SS
1628-
&& (ctxt->mode != X86EMUL_MODE_PROT64 || rpl != cpl))
1629-
|| seg == VCPU_SREG_TR)
1630-
&& null_selector)
1631-
goto exception;
1632-
16331624
/* TR should be in GDT only */
16341625
if (seg == VCPU_SREG_TR && (selector & (1 << 2)))
16351626
goto exception;
16361627

1637-
if (null_selector) /* for NULL selector skip all following checks */
1628+
/* NULL selector is not valid for TR, CS and (except for long mode) SS */
1629+
if (null_selector) {
1630+
if (seg == VCPU_SREG_CS || seg == VCPU_SREG_TR)
1631+
goto exception;
1632+
1633+
if (seg == VCPU_SREG_SS) {
1634+
if (ctxt->mode != X86EMUL_MODE_PROT64 || rpl != cpl)
1635+
goto exception;
1636+
1637+
/*
1638+
* ctxt->ops->set_segment expects the CPL to be in
1639+
* SS.DPL, so fake an expand-up 32-bit data segment.
1640+
*/
1641+
seg_desc.type = 3;
1642+
seg_desc.p = 1;
1643+
seg_desc.s = 1;
1644+
seg_desc.dpl = cpl;
1645+
seg_desc.d = 1;
1646+
seg_desc.g = 1;
1647+
}
1648+
1649+
/* Skip all following checks */
16381650
goto load;
1651+
}
16391652

16401653
ret = read_segment_descriptor(ctxt, selector, &seg_desc, &desc_addr);
16411654
if (ret != X86EMUL_CONTINUE)
@@ -1751,6 +1764,21 @@ static int load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
17511764
u16 selector, int seg)
17521765
{
17531766
u8 cpl = ctxt->ops->cpl(ctxt);
1767+
1768+
/*
1769+
* None of MOV, POP and LSS can load a NULL selector in CPL=3, but
1770+
* they can load it at CPL<3 (Intel's manual says only LSS can,
1771+
* but it's wrong).
1772+
*
1773+
* However, the Intel manual says that putting IST=1/DPL=3 in
1774+
* an interrupt gate will result in SS=3 (the AMD manual instead
1775+
* says it doesn't), so allow SS=3 in __load_segment_descriptor
1776+
* and only forbid it here.
1777+
*/
1778+
if (seg == VCPU_SREG_SS && selector == 3 &&
1779+
ctxt->mode == X86EMUL_MODE_PROT64)
1780+
return emulate_exception(ctxt, GP_VECTOR, 0, true);
1781+
17541782
return __load_segment_descriptor(ctxt, selector, seg, cpl,
17551783
X86_TRANSFER_NONE, NULL);
17561784
}

0 commit comments

Comments
 (0)