[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4/6] xsm: flask: change the interface and default policy for xsm_map_gmfn_foregin
On Wed, 23 Aug 2017, Zhongze Liu wrote: > The original xsm_map_gmfn_foregin policy checks if source domain has the > proper > privileges over the target domain. Under this policy, it's not allowed if a > Dom0 > wants to map pages from one DomU to another, this restricts some useful yet > not > dangerous usages of the API, such as sharing pages among DomU's by calling > XENMEM_add_to_physmap from Dom0. > > Change the policy to: IIF current domain has the proper privilege on the ^ IFF > target domain and source domain, grant the access. > > References to this xsm check have also been updated to pass the current > domain as a new parameter. > > This is for the proposal "Allow setting up shared memory areas between VMs > from xl config file" (see [1]). > > [1] https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg03047.html > > Signed-off-by: Zhongze Liu <blackskygg@xxxxxxxxx> > > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx> > Cc: Julien Grall <julien.grall@xxxxxxx> > Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx> > Cc: Jan Beulich <jbeulich@xxxxxxxx> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> > Cc: xen-devel@xxxxxxxxxxxxx > --- > xen/arch/arm/mm.c | 2 +- > xen/arch/x86/mm/p2m.c | 2 +- > xen/include/xsm/dummy.h | 6 ++++-- > xen/include/xsm/xsm.h | 7 ++++--- > xen/xsm/flask/hooks.c | 6 ++++-- > 5 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index a810a056d7..9ec78d8c03 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -1284,7 +1284,7 @@ int xenmem_add_to_physmap_one( > return -EINVAL; > } > > - rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od); > + rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, d, od); > if ( rc ) > { > rcu_unlock_domain(od); > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c > index e8a57d118c..a547fd00c0 100644 > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -2545,7 +2545,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long > fgfn, > if ( tdom == fdom ) > goto out; > > - rc = xsm_map_gmfn_foreign(XSM_TARGET, tdom, fdom); > + rc = xsm_map_gmfn_foreign(XSM_TARGET, current->domain, tdom, fdom); > if ( rc ) > goto out; > > diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h > index 62fcea6f04..28dbc6f2a2 100644 > --- a/xen/include/xsm/dummy.h > +++ b/xen/include/xsm/dummy.h > @@ -525,10 +525,12 @@ static XSM_INLINE int > xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1, > return xsm_default_action(action, d1, d2); > } > > -static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, > struct domain *t) > +static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *cd, > + struct domain *d, struct domain > *t) > { > XSM_ASSERT_ACTION(XSM_TARGET); > - return xsm_default_action(action, d, t); > + return xsm_default_action(action, cd, d) || > + xsm_default_action(action, cd, t); We need to preserve the returned errors: rc = xsm_default_action(action, cd, d); if (rc) return rc; return xsm_default_action(action, cd, t); > } > > static XSM_INLINE int xsm_hvm_param(XSM_DEFAULT_ARG struct domain *d, > unsigned long op) > diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h > index 60c0fd6a62..a20654a803 100644 > --- a/xen/include/xsm/xsm.h > +++ b/xen/include/xsm/xsm.h > @@ -85,7 +85,7 @@ struct xsm_operations { > int (*memory_pin_page) (struct domain *d1, struct domain *d2, struct > page_info *page); > int (*add_to_physmap) (struct domain *d1, struct domain *d2); > int (*remove_from_physmap) (struct domain *d1, struct domain *d2); > - int (*map_gmfn_foreign) (struct domain *d, struct domain *t); > + int (*map_gmfn_foreign) (struct domain *cd, struct domain *d, struct > domain *t); > int (*claim_pages) (struct domain *d); > > int (*console_io) (struct domain *d, int cmd); > @@ -372,9 +372,10 @@ static inline int xsm_remove_from_physmap(xsm_default_t > def, struct domain *d1, > return xsm_ops->remove_from_physmap(d1, d2); > } > > -static inline int xsm_map_gmfn_foreign (xsm_default_t def, struct domain *d, > struct domain *t) > +static inline int xsm_map_gmfn_foreign (xsm_default_t def, struct domain *cd, > + struct domain *d, struct domain *t) > { > - return xsm_ops->map_gmfn_foreign(d, t); > + return xsm_ops->map_gmfn_foreign(cd, d, t); > } > > static inline int xsm_claim_pages(xsm_default_t def, struct domain *d) > diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c > index 91146275bb..3408b6b9e1 100644 > --- a/xen/xsm/flask/hooks.c > +++ b/xen/xsm/flask/hooks.c > @@ -1165,9 +1165,11 @@ static int flask_remove_from_physmap(struct domain > *d1, struct domain *d2) > return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); > } > > -static int flask_map_gmfn_foreign(struct domain *d, struct domain *t) > +static int flask_map_gmfn_foreign(struct domain *cd, > + struct domain *d, struct domain *t) > { > - return domain_has_perm(d, t, SECCLASS_MMU, MMU__MAP_READ | > MMU__MAP_WRITE); > + return domain_has_perm(cd, d, SECCLASS_MMU, MMU__MAP_READ | > MMU__MAP_WRITE) || > + domain_has_perm(cd, t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE); > } Same here: rc = domain_has_perm(cd, d, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE); if (rc) return rc; return domain_has_perm(cd, t, SECCLASS_MMU, MMU__MAP_READ | MMU__MAP_WRITE); Also, I just want to point out that in the regular case cd and d are one and the same. The code assumes that domain_has_perm returns 0 in that case. I think that is correct, but I don't know enough about XSM to be sure about it. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |