Re: [Xen-devel] [PATCH 3/4 V2] X86: MPX IA32_BNDCFGS msr handle

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

Jan Beulich wrote:
>>>> On 26.11.13 at 11:11, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
>> Jan Beulich wrote:
>>>>>> On 25.11.13 at 09:25, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx>
>>>>>> wrote: 
>>>> +static bool_t bndcfgs_invalid(u64 msr_content)
>>>> +{
>>>> +    /* BNDCFGS MSR reserved bits (11 ~ 2) must be zero */
>>>> +    if ( msr_content & 0xffc )
>>>> +        return 1;
>>>> +
>>>> +    /* Canonical address reserved bits must be zero */
>>>> +    if ( hvm_long_mode_enabled(current) )
>>>> +        /* 48b linear address for x86_64 guest */
>>>> +        return !!(msr_content & (~(u64)0 << 48) );
>>> 
>>> !is_canonical_address()
>> 
>> Not simply is_canonical_address() check, per MPX doc 9.3.5.1,
>> It will #GP if canonical address reserved bits (must be zero) check
>> fails 
> 
> You're checking the reserved bits (2...11) above. For the address
> portion it says "The base of bound directory is a 4K page aligned
> linear address, and is always in canonical form. Any load into BNDCFGx
> (XRSTOR or WRMSR) ensures that the highest implemented bit of
> the linear address is sign extended to guarantee the canonicality
> of this address", which to me means _if_ you check anything here,
> then you want to check the address for being canonical.
> 
>>>> +    else
>>>> +        /* 32b linear address for x86_32 (include PAE) guest */
>>>> +        return !!(msr_content & (~(u64)0 << 32) );
>>> 
>>> !!(msr_content >> 32)

If so, canonical check for x86-32 (and pae) -- bit63~32 should not always be 
zero, but is sign extension of bit 31?

>>> 
>>>> @@ -3010,6 +3025,12 @@ int hvm_msr_read_intercept(unsigned int msr,
>>>>          uint64_t *msr_content) hvm_get_guest_pat(v, msr_content);
>>>> break; 
>>>> 
>>>> +    case MSR_IA32_BNDCFGS:
>>>> +        if ( !cpu_has_mpx )
>>> 
>>> Wasn't it you who started to properly use hvm_cpuid() for cases like
>>> this? We're not after host capabilities here, but after what the
>>> guest is supposed to (not) use.
>> 
>> When malicious or flawed guest access reserved or unimplemented msr,
>> I think inject #GP is better than silently ignore?
> 
> I didn't say "silently ignore"; in fact, I asked you to _tighten_ the
> check (hvm_cpuid() shouldn't return the respective flag set when
> !cpu_has_mpx).
> 
> Jan

Not quite clear your meaning here, hvm_cpuid will not return flag when 
!cpu_has_mpx.

Thanks,
Jinsong
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel