Re: [Xen-devel] [PATCH] x86emul: correct loading of %ss

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/09/16 16:25, Jan Beulich wrote:
>>>> On 26.09.16 at 15:40, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 21/09/16 10:05, Jan Beulich wrote:
>>> - Instead of #NP, #SS needs to be raised for non-present descriptors.
>>> - Loading a null selector is fine in 64-bit mode at CPL != 3, as long
>>>   as RPL == CPL.
>>> - Don't lose the low two selector bits on null selector loads (also
>>>   applies to %ds, %es, %fs, and %gs).
>>>
>>> Since we need CPL earlier now, also switch to using get_cpl() (instead
>>> of open coding it).
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, although...
>>
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -1194,18 +1194,25 @@ protmode_load_seg(
>>>      struct x86_emulate_ctxt *ctxt,
>>>      const struct x86_emulate_ops *ops)
>>>  {
>>> -    struct segment_register desctab, ss;
>>> +    struct segment_register desctab;
>>>      struct { uint32_t a, b; } desc;
>>> -    uint8_t dpl, rpl, cpl;
>>> +    uint8_t dpl, rpl;
>>> +    int cpl = get_cpl(ctxt, ops);
>>>      uint32_t new_desc_b, a_flag = 0x100;
>>>      int rc, fault_type = EXC_GP;
>>>  
>>> +    if ( cpl < 0 )
>>> +        return X86EMUL_UNHANDLEABLE;
>>> +
>>>      /* NULL selector? */
>>>      if ( (sel & 0xfffc) == 0 )
>>>      {
>>> -        if ( (seg == x86_seg_cs) || (seg == x86_seg_ss) )
>>> +        if ( (seg == x86_seg_cs) ||
>>> +             ((seg == x86_seg_ss) &&
>>> +              (!mode_64bit() || (cpl == 3) || (cpl != sel))) )
> I've just noticed that this depends on
>
> @@ -607,7 +609,7 @@ do{ asm volatile (
>  })
>  #define truncate_ea(ea) truncate_word((ea), ad_bytes)
>  
> -#define mode_64bit() (def_ad_bytes == 8)
> +#define mode_64bit() (ctxt->addr_size == 64)
>  
>  #define fail_if(p)                                      \
>  do {                                                    \
>
> from the large decode rework series. I'll assume you're okay with me
> folding this in.

Yeah - that looks fine.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel