Re: [PATCH for-4.5] xsm/flask: add two missing domctls

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 11/25/2014 01:19 PM, Andrew Cooper wrote:
> On 25/11/14 16:57, Daniel De Graaf wrote:
>> Reported-by: Michael Young <m.a.young@durham.ac.uk>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> CC'd Konrad, as this should be accepted into Xen-4.5.  Without it,
> migration/suspend fails with -EPERM in the default case when XSM is
> compiled into Xen.

Thanks, for some reason I blanked on the CC for the freeze exception.

> Daniel: there are 4 hypercalls for getting/setting bits of PV VCPU state:
>
> XEN_DOMCTL_{get,set}vcpucontext
> XEN_DOMCTL_{get,set}_ext_vcpucontext
> XEN_DOMCTL_{get,set}vcpuextstate
> XEN_DOMCTL_{get,set}_vcpu_msrs
>
> I see no reason for these to have separate access vectors; you typically
> either need to use all of them, or none, but I presume it is too late to
> coalesce the vectors in a backwards compatible way?
>
> ~Andrew

Because the security policy in Xen is kept inside the tree, it is
possible to change them in the future - though this is certainly a topic
for v4.6.  This will cause anyone who has defined their own security
policy to need to modify it, but this is already true when new
permissions are being defined, and it is easier to remove permissions
(just fix the policy compile error) than it is to add them (which either
requires thought on who needs to be allowed access to a permission or
testing to discover the AVC denials).  If a custom policy is using the
macros defined in xen.if, these changes will be applied transparently.

I agree combining these four domctls into a single pair of permissions
is useful (retaining the get/set split); I cannot think of any case
where someone might have a use for one type of CPU/register state and at
the same time cannot be trusted with the others.  This would also
simplify future additions of new types of CPU state.

-- 
Daniel De Graaf
National Security Agency