Re: [PATCH] evtchn/Flask: pre-allocate node on send path

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Wed, Sep 30, 2020 at 2:20 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 29.09.2020 19:20, Jason Andryuk wrote:
> > On Mon, Sep 28, 2020 at 3:49 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >> On 25.09.2020 20:08, Jason Andryuk wrote:
> >>>  Also, a domain label can transition (change) at runtime.
> >>> Dropping the send check would latch the permission at bind time which
> >>> would not necessarily be valid for the security policy.
> >>
> >> I did realize this as a possibility too, but there the immediate
> >> question is: Why for interdomain channels, but then not also for
> >> vIRQ-s, for example? In fact, unless I'm overlooking something,
> >> for this specific case there's not even any check in the binding
> >> logic, not even for global vIRQ-s. (After all there are two
> >> aspects in the permissions here: One is to be eligible to send,
> >> which ought to not matter when the sender is Xen, while the
> >> other is permission to learn / know of certain events, i.e. in
> >> particular global vIRQ-s.)
> >
> > I'm not familiar with vIRQ-s, but I did a little bit of review.  A
> > vIRQ source is always Xen and its destination is a domain, correct?
> > They don't allow a data flow between domains,
>
> Yes and yes.
>
> > so maybe that is why they weren't hooked originally?
>
> Not so much, I assume. Looking a little more closely I find that ...
>
> > Hmmm, even for non-XSM, there is no restriction on binding the "dom0"
> > vIRQ-s?
>
> ... while binding is allowed, an event would never be received unless
> the domain was designated as the receiver via
> XEN_DOMCTL_set_virq_handler.

And all of those default to the hardware_domain.  This permission
isn't granted in the default policy, so I was initially confused as to
how it worked.

> >> The fundamental issue here is that the sending path should be
> >> fast and hence lightweight. This means (to me) that in
> >> particular no memory allocations should occur, and (more
> >> generally) no global or domain wide locks should be taken (for
> >> rw ones: no write locks).
> >
> > Yes, that all seems good and reasonable.  With XSM/Flask you also need
> > the AVC entry for send to be lightweight.
> >
> > It wouldn't help with the domain transition case, but you could run
> > the xsm send hooks at bind time to pre-populate the cache.
>
> Question is for how long such an entry would remain in the cache,
> i.e. whether pre-filling is useful at all. After all pre-filling
> has the downside of potentially masking real issues when testing
> (as opposed to running in the wild).

Yes, good point.

> >  That would
> > still require avc code to bypass the memory allocation when holding a
> > lock in case the entry isn't found.  Your preallocation idea could be
> > generalized to have avc maintain a reserve of nodes for use when it
> > cannot allocate.  When it can allocate, it would refill the reserve in
> > addition to whatever regular allocation it would perform.  But if it's
> > only evtchn_send that needs special handling, then the complexity may
> > not be worth adding.
>
> It was this last aspect which made me not introduce a general
> mechanism.

I think we go with this patch since it doesn't restrict the use of XSM
and tries to populate the avc.

Tested-by: Jason Andryuk <jandryuk@xxxxxxxxx>

Thanks,
Jason