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

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 11/25/2013 07:32 PM, Mukesh Rathor wrote:
> On Mon, 25 Nov 2013 14:00:35 -0500
> Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
>
> > On 11/25/2013 04:03 AM, Jan Beulich wrote:
> > > > > > On 23.11.13 at 01:03, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> > > > > > 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

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