Re: [V2 PATCH 7/8] pvh dom0: Add and remove foreign pages

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

On 11/25/2013 07:32 PM, Mukesh Rathor wrote:
> On Mon, 25 Nov 2013 14:00:35 -0500
> Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
>
>> On 11/25/2013 04:03 AM, Jan Beulich wrote:
>>>>>> On 23.11.13 at 01:03, Mukesh Rathor <mukesh.rathor@oracle.com>
>>>>>> wrote:
>>>> +static int xenmem_add_foreign_to_pmap(unsigned long fgfn,
>>>> unsigned long gpfn,
>>>> +                                      domid_t foreign_domid)
>>>> +{
>>>> +    p2m_type_t p2mt, p2mt_prev;
>>>> +    int rc = 0;
>>>> +    unsigned long prev_mfn, mfn = 0;
>>>> +    struct domain *fdom, *currd = current->domain;
>>>> +    struct page_info *page = NULL;
>>>> +
>>>> +    if ( currd->domain_id == foreign_domid || foreign_domid ==
>>>> DOMID_SELF ||
>>>> +         !is_pvh_domain(currd) )
>>>> +        return -EINVAL;
>>>> +
>>>> +    if ( !is_control_domain(currd) ||
>>>> +         (fdom = get_pg_owner(foreign_domid)) == NULL )
>>>> +        return -EPERM;
>>>
>>> Is this the right approach (i.e. shouldn't this be an XSM call)?
>>> Cc-ing Daniel...
>>>
>>
>> Yes, this should be an XSM call; it needs to explicitly check if currd
>> has the right to access pages from fdom.
>
> I thought the control_domain would always have right to access pages
> from fdoms.

This is true unless you are creating a system with multiple control domains
or with a control domain that gives up privileges after setting up some
initial boot domains that contain secrets (disk/network encryption, vTPMs,
etc). Preventing the control domain from being able to access pages in such
domains means a compromised control domain does not compromise the entire
system.

> If no, can you please give some hints on which
> xsm call I need to use. Glancing at xsm file, I can't figure quickly....
>
> thanks
> mukesh

This will either need a new XSM hook or a change to the prototype of the
xsm_add_to_physmap hook to add a new parameter for the foreign domain; the
latter seems the simplest change, passing NULL for pg_src when not using
XENMAPSPACE_gmfn_foreign. The hook would look something like this:

int xsm_add_to_physmap(XSM_DEFAULT_ARG struct domain *curr,
		struct domain *target, struct domain *pg_src)
{
     int rc;
     XSM_ASSERT_ACTION(XSM_TARGET);
     rc = xsm_default_action(action, curr, target);
     if ( pg_src && !rc )
         rc = xsm_default_action(action, curr, pg_src);
     return rc;
}
with the corresponding FLASK hook:
{ ...
	rc = domain_has_perm(curr, target, SECCLASS_MMU, MMU__PHYSMAP);
	if ( pg_src && !rc )
	rc = domain_has_perm(curr, pg_src, SECCLASS_MMU, MMU__MAP_READ|MMU__MAP_WRITE);
}

This will require pulling the get_pg_owner(foreign_domid) up a few levels
in order to have the struct domain* available instead of the domid, but
that doesn't seem like it would cause any issues.

-- 
Daniel De Graaf
National Security Agency