Xen devel - May 2013 - XSM dummy policy blocking event channel creation

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

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.

-- 
Daniel De Graaf
National Security Agency

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.

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




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

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 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.
> 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?

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).

-- 
Daniel De Graaf
National Security Agency

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.
>
> 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.
>> 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.
>
> 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


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

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.
>>
>> 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).
>>> 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

-- 
Daniel De Graaf
National Security Agency

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

On 05/02/2013 11:37 AM, Ross Philipson wrote:
> 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.
That would work; it looks like it would be easier than pulling the XSM hook
out of the function. The important part is that the user-accessible caller
still needs to execute the hook. Other callers bypassing the permission
check will need similar justification for doing that, but that discussion
should wait until those additional callers exist.
  
>>>> 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
-- 
Daniel De Graaf
National Security Agency

On 05/02/2013 12:03 PM, Daniel De Graaf wrote:
> That would work; it looks like it would be easier than pulling the XSM hook
> out of the function. The important part is that the user-accessible caller
> still needs to execute the hook. Other callers bypassing the permission
> check will need similar justification for doing that, but that discussion
> should wait until those additional callers exist.
Thanks for your help. I implemented that and it is working nicely.


-- 
Ross Philipson