Skip to content

Commit

Permalink
KVM: x86: fix emulation of "MOV SS, null selector"
Browse files Browse the repository at this point in the history
commit 33ab911 upstream.

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 <zhangxiaohan1@huawei.com>
Fixes: 79d5b4c
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
bonzini authored and gregkh committed Jan 19, 2017
1 parent 1e26cec commit 7718ffc
Showing 1 changed file with 38 additions and 10 deletions.
48 changes: 38 additions & 10 deletions arch/x86/kvm/emulate.c
Original file line number Diff line number Diff line change
Expand Up @@ -1544,7 +1544,6 @@ static int write_segment_descriptor(struct x86_emulate_ctxt *ctxt,
&ctxt->exception);
}

/* Does not support long mode */
static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
u16 selector, int seg, u8 cpl,
enum x86_transfer_type transfer,
Expand Down Expand Up @@ -1581,20 +1580,34 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,

rpl = selector & 3;

/* NULL selector is not valid for TR, CS and SS (except for long mode) */
if ((seg == VCPU_SREG_CS
|| (seg == VCPU_SREG_SS
&& (ctxt->mode != X86EMUL_MODE_PROT64 || rpl != cpl))
|| seg == VCPU_SREG_TR)
&& null_selector)
goto exception;

/* TR should be in GDT only */
if (seg == VCPU_SREG_TR && (selector & (1 << 2)))
goto exception;

if (null_selector) /* for NULL selector skip all following checks */
/* NULL selector is not valid for TR, CS and (except for long mode) SS */
if (null_selector) {
if (seg == VCPU_SREG_CS || seg == VCPU_SREG_TR)
goto exception;

if (seg == VCPU_SREG_SS) {
if (ctxt->mode != X86EMUL_MODE_PROT64 || rpl != cpl)
goto exception;

/*
* ctxt->ops->set_segment expects the CPL to be in
* SS.DPL, so fake an expand-up 32-bit data segment.
*/
seg_desc.type = 3;
seg_desc.p = 1;
seg_desc.s = 1;
seg_desc.dpl = cpl;
seg_desc.d = 1;
seg_desc.g = 1;
}

/* Skip all following checks */
goto load;
}

ret = read_segment_descriptor(ctxt, selector, &seg_desc, &desc_addr);
if (ret != X86EMUL_CONTINUE)
Expand Down Expand Up @@ -1710,6 +1723,21 @@ static int load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
u16 selector, int seg)
{
u8 cpl = ctxt->ops->cpl(ctxt);

/*
* None of MOV, POP and LSS can load a NULL selector in CPL=3, but
* they can load it at CPL<3 (Intel's manual says only LSS can,
* but it's wrong).
*
* However, the Intel manual says that putting IST=1/DPL=3 in
* an interrupt gate will result in SS=3 (the AMD manual instead
* says it doesn't), so allow SS=3 in __load_segment_descriptor
* and only forbid it here.
*/
if (seg == VCPU_SREG_SS && selector == 3 &&
ctxt->mode == X86EMUL_MODE_PROT64)
return emulate_exception(ctxt, GP_VECTOR, 0, true);

return __load_segment_descriptor(ctxt, selector, seg, cpl,
X86_TRANSFER_NONE, NULL);
}
Expand Down

0 comments on commit 7718ffc

Please sign in to comment.