From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ross Philipson Subject: Re: XSM dummy policy blocking event channel creation Date: Thu, 2 May 2013 11:37:24 -0400 Message-ID: <51828834.4010501@citrix.com> References: <51816FA9.9020402@citrix.com> <51818000.2070108@tycho.nsa.gov> <51818498.9040708@citrix.com> <518190EC.2030104@tycho.nsa.gov> <518276E4.6010602@citrix.com> <518281CC.6000101@tycho.nsa.gov> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <518281CC.6000101@tycho.nsa.gov> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Daniel De Graaf Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 05/02/2013 11:10 AM, Daniel De Graaf wrote: > On 05/02/2013 10:23 AM, Ross Philipson wrote: >> On 05/01/2013 06:02 PM, Daniel De Graaf wrote: >>> On 05/01/2013 05:09 PM, Ross Philipson wrote: >>>> On 05/01/2013 04:50 PM, Daniel De Graaf wrote: >>>>> On 05/01/2013 03:40 PM, Ross Philipson wrote: >>>>>> I am working on the next set of V4V patches to post to the list. I >>>>>> have pulled the very latest staging branch of Xen and I quickly ran >>>>>> into a new problem. We are basically trying to create an event >>>>>> channel >>>>>> during the creation of dom0. We have split up the >>>>>> evtchn_alloc_unbound() function into two functions but the basic >>>>>> problem is the same. The call to xsm_evtchn_unbound() is returning >>>>>> -EPERM from the new code in xsm/dummy.h. This patch set added this >>>>>> functionality: >>>>>> >>>>>> http://lists.xen.org/archives/html/xen-devel/2012-11/msg01920.html >>>>>> >>>>>> Specifically we are failing this part of the test return -EPERM: >>>>>> >>>>>> static always_inline int xsm_default_action( >>>>>> xsm_default_t action, struct domain *src, struct domain *target) >>>>>> { >>>>>> ... >>>>>> case XSM_TARGET: >>>>>> if ( src != target && !IS_PRIV_FOR(src, target) ) >>>>>> return -EPERM; >>>>>> >>>>>> The src domain is the current->domain which is idle_domain and target >>>>>> is dom0 which is in the process of being created. Neither of them is >>>>>> privileged (dom0 is not set to privileged yet). And I have not gotten >>>>>> past dom0 creation yet so I don't know what will happen when V4V >>>>>> tries >>>>>> to initialize for domU's. >>>>>> >>>>>> I need some advice on how to proceed here. I am not terribly >>>>>> conversant in the working of XSM and do not have a clear idea how to >>>>>> proceed. >>>>>> >>>>>> Thanks, >>>>>> Ross Philipson >>>>> >>>>> The answer for domUs is actually easier: the domain builder (dom0) >>>>> will >>>>> satisfy IS_PRIV_FOR(dom0, newdomU) and so the creation will be >>>>> allowed. >>>>> With FLASK, the remote domain will also be considered, so I presume >>>>> you >>>>> have set something valid there (it needs to have a struct domain* that >>>>> rcu_lock_domain_by_any_id can be used to fetch). >>>>> >>>>> Event channels created by the hypervisor are normally allocated using >>>>> alloc_unbound_xen_event_channel instead of evtchn_alloc_unbound; the >>>>> former will not fail the creation of the event channel if the XSM hook >>>>> denies, but will instead change the remote domain ID to DOMID_INVALID. >>>>> Depending on what you intend the remote domid to be in this newly >>>>> allocated event channel, this may or may not be useful behavior; the >>>>> semantics of alloc_unbound_xen_event_channel are also different, so >>>>> you would need to change your calling code with this in mind. >>>>> >>>>> If you want to use evtchn_alloc_unbound directly, you probably need to >>>>> add logic to omit the XSM check during the creation of dom0 - and this >>>>> likely belongs in the evtchn_alloc_unbound function rather than the >>>>> dummy XSM hook. The XSM changes you referenced don't actually >>>>> introduce >>>>> this, since before the call to rcu_lock_target_domain_by_id would have >>>>> failed because the idle domain does not satisfy IS_PRIV_FOR on dom0. >>>>> >>>> Thank you for getting back to me so quickly. >>>> >>>> Perhaps I should share the new split up evtchn_alloc* functions since >>>> the way they are done is relevant to your answer. I attached a snippet >>>> with the split functions. We are directly calling >>>> evtchn_alloc_unbound_domain() which does not call >>>> rcu_lock_domain_by_any_id(). So the permissions block is related only >>>> to the changes in dummy.c I believe. >>> >>> Well, the way you have it written happens to avoid the permission check, >>> presumably because the hypervisor is doing the creation. I can't tell if >>> this is still correct in the new function since I can't see who calls >>> it; >>> if this is only called during domain creation and remote_domid is always >>> something special (DOMID_SELF, perhaps?) then completely removing the >>> XSM >>> check would be justified for internal callers. >> >> The very first time the call is made to create the event channel in >> V4V, the target is dom0 and the src (which is current->domain) is the >> idle_domain. This is during creation of dom0 and this is the case that >> fails the XSM check. Maybe this is the one we can special case and >> bypass the XSM hook. >> >> All subsequent calls that I see are either src == dom0 and target is >> dom0 or some other domU which of course passes. I also see cases where >> src == target for both dom0 and domU's which also pass the test. >> >> In the case of V4V calling evtchn_alloc_unbound_domain(), remote_domid >> is always the same as the domid which becomes the target domid in the >> XSM calls. I am attaching the relevant v4v_init() code for clarity. > > Actually, I think the call to evtchn_alloc_unbound_domain in v4v_init > should always bypass the XSM hook. The invocation of this function is > either during dom0 creation (which is always allowed) or has already > passed the access check for domain creation. Since this event channel > is always bound to the local domain, there is also no need for a FLASK > check for creating a new communication path. This would also avoid > special-casing the idle domain's creation of event channels. Ok that seems to make sense to me. Would a boolean flag passed to evtchn_alloc_unbound_domain() be the way to go in case something else in the future wants to use that routine? Also evtchn_alloc_unbound() calls to the second routine and that call always used the XSM call before. > >>> >>> The goal of the XSM check here is to control the creation of event >>> channels: >>> we don't want a domain creating event channels for another unless it >>> controls >>> that domain (the check between current->domain and d that you have hit >>> here), >>> and with FLASK we also want to control what domains can communicate with >>> each >>> other (which is the reason remote_domid is passed to the XSM hook). V4V >>> will >>> require its own XSM hooks to satisfy the second goal, and the first does >>> not >>> apply when the hypervisor is the entity creating the event channel. >>> >> >> What I am confused about here is that the remote_domid that is passed >> to evtchn_alloc_unbound_domain() is then passed to >> xsm_evtchn_unbound() as id2 but is never used in the XSM code. > > It's not used in the dummy XSM module, but it is used in FLASK; see the > flask_evtchn_unbound function in xen/xsm/flask/hooks.c, which is called > by xsm_evtchn_unbound if using FLASK. If V4V ends up calling this XSM > hook before the target domain is inserted into the domain hash, the > prototype for the function will need to be changed to take a struct > domain* instead of a domid for the target (or FLASK will always deny the > creation because it cannot determine the target). Make sense, thanks. > >>>> I am still digging through the history of the posts and responses to >>>> see how they settled on that configuration of the evtchn_alloc* >>>> functions but this might explain things for you a bit better (e.g. why >>>> it worked prior to the dummy.c changes). >>>> >>>> Thanks, >>>> Ross >>> >>> What is this event channel being used for - is it similar to the HVM >>> ioreq >>> event channel created on a domain that someone else can bind and send >>> to, or >>> a VIRQ implemented as an unbound event channel, or something else? >>> What are >>> you passing as remote_domid here? >> >> Yes it is like a VIRQ implemented as a unbound event channel. Each >> domain has a v4v structure associated with it. The event channel is >> used to signal a guess from within Xen V4V bits. It used to actually >> be a VIRQ but we were asked to change it to its current form. > > OK, that makes sense. > >>> The V4V-like code that I am currently using uses a VIRQ for this >>> purpose, as >>> does the last V4V patch RFC that I checked (dated 2012-05-31). >>> >> >> Thanks >> Ross > > -- Ross Philipson