On 27/07/18 10:28, Jan Beulich wrote:
>>>> On 19.07.18 at 13:44, <[email protected]> wrote:
>> It turns out that Xen has never enforced that a domain remain within the
>> xstate features advertised in CPUID.
>>
>> The check of new_bv against xfeature_mask ensures that a domain stays within
>> the set of features that Xen has enabled in hardware (and therefore isn't a
>> security problem), but this does means that attempts to level a guest for
>> migration safety might not be effective if the guest ignores CPUID.
>>
>> Check the CPUID policy in validate_xstate() (for incoming migration) and in
>> handle_xsetbv() (for guest XSETBV instructions). This subsumes the PKRU
>> check
>> for PV guests in handle_xsetbv() (and also demonstrates that I should have
>> spotted this problem while reviewing c/s fbf9971241f).
>>
>> For migration, this is correct despite the current (mis)ordering of data
>> because d->arch.cpuid is the applicable max policy.
>>
>> Signed-off-by: Andrew Cooper <[email protected]>
>> ---
>> CC: Jan Beulich <[email protected]>
>>
>> v2:
>> * Leave valid_xcr0() alone and duplicate the checks in validate_xstate() and
>> handle_xsetbv().
>> v3:
>> * Note the migration safety in the commit message.
>>
>> Backporting notes: This is safe in the restore case, but only back as far as
>> the introduction of cpuid_policy infrastructure. Before then, a restore
>> boolean needs to be plumbed down as well, and used to select between the
>> hardware maximum value and calls to {hvm,pv}_cpuid() to find the
>> toolstack-chosen level.
> While trying to determine the exact boundary here (looks like it's
> between 4.7 and 4.8, in which case the remark is relevant only for
> people maintaining releases no longer fully XenProject maintained)
> I've become confused by the reference to {hvm,pv}_cpuid() above:
> Is this simply an ordering concern? If so, the bounding the two
> functions do would need to be replicated (or better shared) I think,
> if the sole reason for otherwise using the HW maximum is that
> d->arch.cpuids[] isn't populated yet.
Looking over things, 4.9 is fine because that was when cpuid_policy was
introduced.
Before 4.9, the calls to {hvm,pv}_cpuid() are needed to because the
information can't be read directly out of d->arch.cpuids[]. The restore
boolean is needed because this array will be empty at the time it is
accessed on the restore path.
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
