Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1587616

[PATCH 3.16 04/12] KVM: x86: fix emulation of "MOV SS, null selector"

From Ben Hutchings <ben@decadent.org.uk>
Newsgroups linux.kernel
Subject [PATCH 3.16 04/12] KVM: x86: fix emulation of "MOV SS, null selector"
Date 2017-02-24 14:20 +0100
Message-ID <tenWV-FB-11@gated-at.bofh.it> (permalink)
References <ten0R-8s6-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


3.16.41-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Paolo Bonzini <pbonzini@redhat.com>

commit 33ab91103b3415e12457e3104f0e4517ce12d0f3 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: 79d5b4c3cd809c770d4bf9812635647016c56011
Cc: stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 arch/x86/kvm/emulate.c | 48 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 10 deletions(-)

--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1439,7 +1439,6 @@ static int write_segment_descriptor(stru
 				    &ctxt->exception);
 }
 
-/* Does not support long mode */
 static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
 				     u16 selector, int seg, u8 cpl,
 				     bool in_task_switch,
@@ -1475,20 +1474,34 @@ static int __load_segment_descriptor(str
 
 	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)
@@ -1584,6 +1597,21 @@ static int load_segment_descriptor(struc
 				   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, false, NULL);
 }
 

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 3.16 00/12] 3.16.41-rc1 review Ben Hutchings <ben@decadent.org.uk> - 2017-02-24 14:10 +0100
  [PATCH 3.16 12/12] tcp: avoid infinite loop in tcp_splice_read() Ben Hutchings <ben@decadent.org.uk> - 2017-02-24 14:20 +0100
  [PATCH 3.16 04/12] KVM: x86: fix emulation of "MOV SS, null selector" Ben Hutchings <ben@decadent.org.uk> - 2017-02-24 14:20 +0100
  [PATCH 3.16 07/12] USB: serial: kl5kusb105: fix line-state error  handling Ben Hutchings <ben@decadent.org.uk> - 2017-02-24 14:20 +0100
  [PATCH 3.16 06/12] selinux: fix off-by-one in setprocattr Ben Hutchings <ben@decadent.org.uk> - 2017-02-24 14:20 +0100
  [PATCH 3.16 02/12] kvm: nVMX: Allow L1 to intercept software  exceptions (#BP and #OF) Ben Hutchings <ben@decadent.org.uk> - 2017-02-24 14:20 +0100
  [PATCH 3.16 01/12] mnt: Add a per mount namespace limit on the  number of mounts Ben Hutchings <ben@decadent.org.uk> - 2017-02-24 14:20 +0100
  [PATCH 3.16 03/12] ext4: validate s_first_meta_bg at mount time Ben Hutchings <ben@decadent.org.uk> - 2017-02-24 14:20 +0100
  Re: [PATCH 3.16 00/12] 3.16.41-rc1 review Guenter Roeck <linux@roeck-us.net> - 2017-02-24 17:30 +0100

csiph-web