[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH 1/1] xsm: allows system domains to allocate evtchn


  • To: Julien Grall <julien@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>
  • From: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Date: Wed, 30 Mar 2022 09:05:32 -0400
  • Arc-authentication-results: i=1; mx.zohomail.com; dkim=pass header.i=apertussolutions.com; spf=pass smtp.mailfrom=dpsmith@xxxxxxxxxxxxxxxxxxxx; dmarc=pass header.from=<dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1648645564; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=bUJmJCGEdM2uqDh5Muuv5gI6Wx4z05uTf9ucNEM+z7o=; b=EFP7NIwMh4s0TUEQMEAPD5+dWlMzSV5rX21Uph3DPPnbjHBQMp5H1rNhFNx0TApazozBk/WQEvT5IBUuIwRv4USEJ19SmZF46kEhvbbI/VpQwiU/KvIzk4Ca4P01xDWfL1Bz4QgCklle3cKgxMSvbw01jiC/vav4iKNHX2BlCPQ=
  • Arc-seal: i=1; a=rsa-sha256; t=1648645564; cv=none; d=zohomail.com; s=zohoarc; b=nXqD72tNIieKIJnJ+jJfnVFBS/+HTVnM/t5eCHxT8+29GGJSWgE08uWeDoiHBINyPwOMjPqWWXMOpljT3b/nVr6lXaViIUrJTyp8ISAgC/jqsXCYPRiRbYQ6nXek5Lt/3d+t1A6ULgaujDx2KEEDrW+crL7ZMjXM3BBDW/J9MoM=
  • Cc: scott.davis@xxxxxxxxxx, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Jason Andryuk <jandryuk@xxxxxxxxx>
  • Delivery-date: Wed, 30 Mar 2022 13:06:28 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hey Julien,

On 3/29/22 17:57, Julien Grall wrote:
> Hi Daniel,
> 
> On 29/03/2022 19:57, Daniel P. Smith wrote:
>> On 3/29/22 02:43, Jan Beulich wrote:
>>> On 28.03.2022 22:36, Daniel P. Smith wrote:
>>>> During domain construction under dom0less and hyperlaunch it is
>>>> necessary to
>>>> allocate at least the event channel for xenstore and potentially the
>>>> event
>>>> channel for the core console. When dom0less and hyperlaunch are
>>>> doing their
>>>> construction logic they are executing under the idle domain context.
>>>> The idle
>>>> domain is not a privileged domain, it is not the target domain, and
>>>> as a result
>>>> under the current default XSM policy is not allowed to allocate the
>>>> event
>>>> channel.
>>>
>>> I appreciate the change is only needed there right now, but it feels
>>> inconsistent. _If_ it is to remain that way, at least a comment needs
>>> to be put in xsm_evtchn_unbound() making clear why this is a special
>>> case, and hence clarifying to people what the approximate conditions
>>> are to have such also put elsewhere. But imo it would be better to
>>> make the adjustment right in xsm_default_action(), without touching
>>> event_channel.c at all. Iirc altering xsm_default_action() was
>>> discussed before, but I don't recall particular reasons speaking
>>> against that approach.
>>
>> By inconsistent, I take it you mean this is first place within an XSM
>> hook where an access decision is based on the current domain being a
>> system domain? I do agree and would add a comment to the change in the
>> XSM hook in a non-RFC version of the patch.
>>
>> As to moving the check down into xsm_default_action(), the concern I
>> have with doing so is that this would then make every XSM check succeed
>> if `current->domain` is a system domain. Doing so would require a review
>> of every function which has an XSM hoook to evaluate every invocation of
>> those functions that,
>>    1. is there ever a time when current->domain may be a system domain
>>    2. if so,
>>      a. is the invocation on behalf of the system domain
>>      b. or is the invocation on behalf of a non-system domain
>>
>> If there is any instance of 2b, then an inadvertent privilege escalation
>> can occur on that path. For evtchn_alloc_unbound() I verified the only
>> place, besides the new hyperlaunch calls, it is invoked is in the evtchn
>> hypercall handler, where current should be pointing at the domain that
>> made the hypercall.
> Auditing existing calls is somewhat easy. The trouble are for new calls.
> I would say they are unlikely, but we would need to rely on the
> reviewers to spot any misuse. So this is a bit risky.
> 
> I am also a bit worry that we would end up to convert a lot of
> XSM_TARGET to XSM_HOOK (Note I have Live-Update in mind). This would
> make more difficult to figure what would the XSM calls allows without
> looking at the helper.

This approach was not mean to be the long term solution but to deal with
the immediate need as I agree doing this long term would make the
default policy very nuanced.

> I quite like the proposal from Roger. If we define two helpers (e.g.
> xsm_{enable, disable}_build_domain()), we could elevate the privilege
> for the idle domain for a short period of time (this could be restricted
> to when the dummy policy is used).

This is where I was initially going but I am hesitant to change the XSM
API in what might be temporary API calls which have a good chance will
be displaced. That being said, and it does seem like there is more in
favor of it, if the priv escalation is the overall preferred approach I
would still agree and prefer it be done in the XSM API so any usage is
more easily tracked.

v/r,
dps



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.