ACK: [SRU][Trusty][PATCH 1/1] KVM: x86: fix emulation of "MOV SS, null selector"

[Stefan Bader]

On 24.07.2018 17:25, Kleber Sacilotto de Souza wrote:
> From: Paolo Bonzini <pbonzini at 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 at huawei.com>
> Fixes: 79d5b4c3cd809c770d4bf9812635647016c56011
> Cc: stable at nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> 
> CVE-2017-2583
> (backported from commit 33ab91103b3415e12457e3104f0e4517ce12d0f3)
> [ klebers: adjusted context. ]
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
Acked-by: Stefan Bader <stefan.bader at 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);
>  }
>  
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20180725/f47a1699/attachment.sig>