>>> On 05.08.16 at 13:20, <jbeul...@suse.com> wrote: Daniel,
I've only now realized that I forgot to Cc you on this v2. Jan > Qemu needs access to this for the domain it controls, both due to it > being used by xc_domain_memory_mapping() (which qemu calls) and the > explicit use in hw/xenpv/xen_domainbuild.c:xen_domain_poll(). Extend > permissions to that of any "ordinary" domctl: A domain controlling the > targeted domain can invoke this operation for that target domain (which > is being achieved by no longer passing NULL to xsm_domctl()). > > This at once avoids a for_each_domain() loop when the ID of an > existing domain gets passed in. > > Reported-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com> > Signed-off-by: Jan Beulich <jbeul...@suse.com> > --- > v2: Add a comment. Clarify description as to what additional permission > is being granted. > --- > I know there had been an alternative patch suggestion, but that one > doesn't seem have seen a formal submission so far, so here is my > original proposal. > > I wonder what good the duplication of the returned domain ID does: I'm > tempted to remove the one in the command-specific structure. Does > anyone have insight into why it was done that way? > > I further wonder why we have XSM_OTHER: The respective conversion into > other XSM_* values in xsm/dummy.h could as well move into the callers, > making intentions more obvious when looking at the actual code. > > --- a/tools/flask/policy/modules/xen.if > +++ b/tools/flask/policy/modules/xen.if > @@ -149,7 +149,7 @@ define(`device_model', ` > create_channel($2, $1, $2_channel) > allow $1 $2_channel:event create; > > - allow $1 $2_target:domain shutdown; > + allow $1 $2_target:domain { getdomaininfo shutdown }; > allow $1 $2_target:mmu { map_read map_write adjust physmap target_hack > }; > allow $1 $2_target:hvm { getparam setparam trackdirtyvram hvmctl > irqlevel > pciroute pcilevel cacheattr send_irq }; > ') > --- a/xen/common/domctl.c > +++ b/xen/common/domctl.c > @@ -396,14 +396,13 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe > switch ( op->cmd ) > { > case XEN_DOMCTL_createdomain: > - case XEN_DOMCTL_getdomaininfo: > case XEN_DOMCTL_test_assign_device: > case XEN_DOMCTL_gdbsx_guestmemio: > d = NULL; > break; > default: > d = rcu_lock_domain_by_id(op->domain); > - if ( d == NULL ) > + if ( !d && op->cmd != XEN_DOMCTL_getdomaininfo ) > return -ESRCH; > } > > @@ -817,14 +816,22 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe > > case XEN_DOMCTL_getdomaininfo: > { > - domid_t dom = op->domain; > - > - rcu_read_lock(&domlist_read_lock); > + domid_t dom = DOMID_INVALID; > > - for_each_domain ( d ) > - if ( d->domain_id >= dom ) > + if ( !d ) > + { > + ret = -EINVAL; > + if ( op->domain >= DOMID_FIRST_RESERVED ) > break; > > + rcu_read_lock(&domlist_read_lock); > + > + dom = op->domain; > + for_each_domain ( d ) > + if ( d->domain_id >= dom ) > + break; > + } > + > ret = -ESRCH; > if ( d == NULL ) > goto getdomaininfo_out; > @@ -839,6 +846,10 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe > copyback = 1; > > getdomaininfo_out: > + /* When d was non-NULL upon entry, no cleanup is needed. */ > + if ( dom == DOMID_INVALID ) > + break; > + > rcu_read_unlock(&domlist_read_lock); > d = NULL; > break; > --- a/xen/include/xsm/dummy.h > +++ b/xen/include/xsm/dummy.h > @@ -61,7 +61,12 @@ static always_inline int xsm_default_act > return 0; > case XSM_TARGET: > if ( src == target ) > + { > return 0; > + case XSM_XS_PRIV: > + if ( src->is_xenstore ) > + return 0; > + } > /* fall through */ > case XSM_DM_PRIV: > if ( target && src->target == target ) > @@ -71,10 +76,6 @@ static always_inline int xsm_default_act > if ( src->is_privileged ) > return 0; > return -EPERM; > - case XSM_XS_PRIV: > - if ( src->is_xenstore || src->is_privileged ) > - return 0; > - return -EPERM; > default: > LINKER_BUG_ON(1); > return -EPERM; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel