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

[Julien Grall <julien@xxxxxxx>]

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

--
Julien Grall