Re: [PATCH v2 10/13] libx86: introduce a helper to deserialise msr_policy objects

["Jan Beulich" <JBeulich@suse.com>]

>>> On 17.07.18 at 18:06, <andrew.cooper3@citrix.com> wrote:
> On 17/07/18 13:01, Jan Beulich wrote:
>>>>> +                goto err;
>>>>> +
>>>>> +            p->plaform_info.raw = data.val;
>>>> No other sanity checking?
>>> Correct.  This is a data marshalling function, not an auditing function.
>>>
>>> The auditing functions are also needed for in-place modification to an
>>> existing policy.
>> Right, but the primary problem with understanding whether the lack
>> of checking here is a problem is the lack of a caller of this function.
> 
> The reason there is no caller is because you objected to my stub
> implementation in v1.
> 
> This marshalling support is currently blocking other work, which is why
> I've split it out, to allow development to continue in parallel.
> 
>> As I think I did say in the earlier reply - it matters quite a bit where p
>> points.
> 
> No - it doesn't.  This is a function to convert data between two binary
> representations, as is explained by its documentation.
> 
> Auditing the contents of the data would a) need to happen in combination
> with a cpuid_policy object, and b) would be a layering violation.

I understand what you mean, but that wasn't my point. If p was the
policy of a live domain, blindly putting something in there without
auditing would (a) make unrolling in case of error impossible and (b)
would transiently show too wide a policy to the guest. Yes, you mean
to disallow policy updates once a guest was started, but this again is
not visible here. As in so many other cases - introducing functions
without callers is problematic.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel