Re: [Xen-devel] [PATCH] x86: correct CPUID output for out of bounds input

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/09/16 10:51, Jan Beulich wrote:
>>>> On 05.09.16 at 11:43, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 05/09/16 07:32, Jan Beulich wrote:
>>>>>> On 02.09.16 at 17:14, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> On 01/09/16 16:27, Jan Beulich wrote:
>>>>>>> +        {
>>>>>>> +            if ( d->arch.x86_vendor == X86_VENDOR_AMD )
>>>>>>> +            {
>>>>>>> +                *eax = 0;
>>>>>>> +                *ebx = 0;
>>>>>>> +                *ecx = 0;
>>>>>>> +                *edx = 0;
>>>>>>> +                return;
>>>>>>> +            }
>>>>>>> +            if ( input >> 16 )
>>>>>>> +                hvm_cpuid(0, &lvl, NULL, NULL, NULL);
>>>>>> Is this really the right way round?  The AMD method of "reserved always
>>>>>> as zero" is the more sane default to take.
>>>>> If anything I'd then say let's _always_ follow the AMD model.
>>>> It would certainly be better to default to AMD, and special case others
>>>> on an as-needed basis.
>>>>
>>>> Strictly speaking, following the AMD model is compatible with the
>>>> "Reserved" nature specified for Intel.
>>>>
>>>> Lets just go with this.
>>> Done. But before sending v2, just to be clear: The group check
>>> which you also didn't like won't go away. That's because if we didn't
>>> do it, we'd hide all CPUID info outside the basic and extended group,
>>> in particular (in case we run virtualized ourselves) and leaves coming
>>> from the lower level hypervisor (most notably our own ones, if it's
>>> another Xen underneath).
>> Architecturally speaking, it is not ok for any of our guests to be able
>> to see our hypervisors leaves.
> I'm not convinced, and even less so by the generalization to groups
> like the Cyrix/VIA or Transmeta ones. Yes, some of the leaves there
> would likely need white listing to be applied just like what we do for
> some of the base and extended leaves. But even for those (base
> and extended) we don't hide everything (in particular we don't hide
> the respective lowest numbered subleaf), and hence I don't see why
> we should do so with all of the other groups.

Xen's handling is still broken (albeit it less than it used to be).  A
guest running under Xen should not be able to find any information not
explicitly controlled by Xen, because information leakage like this
causes problems on migrate, etc.

If at some point in the future we choose to extend the whitelist to
include new groups, that is fine.  However, groups which aren't already
explicitly handled by Xen should be excluded from guest view.

~Andrew

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