Message ID | 20180724152542.12420-2-kleber.souza@canonical.com |
---|---|
State | New |
Headers | show |
Series | Fix for CVE-2017-2583 | expand |
On 24.07.2018 17:25, Kleber Sacilotto de Souza wrote: > From: Paolo Bonzini <pbonzini@redhat.com> > > 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> > > CVE-2017-2583 > (backported from commit 33ab91103b3415e12457e3104f0e4517ce12d0f3) > [ klebers: adjusted context. ] > Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > arch/x86/kvm/emulate.c | 48 +++++++++++++++++++++++++++++++++--------- > 1 file changed, 38 insertions(+), 10 deletions(-) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index d924843b0a78..0bc3a88547d0 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -1454,7 +1454,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, > struct desc_struct *desc) > @@ -1489,20 +1488,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) > @@ -1604,6 +1617,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, NULL); > } > >
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index d924843b0a78..0bc3a88547d0 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -1454,7 +1454,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, struct desc_struct *desc) @@ -1489,20 +1488,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) @@ -1604,6 +1617,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, NULL); }