Re: [Xen-devel] [PATCH 15/25] argo: implement the sendv op

[Christopher Clark <christopher.w.clark@xxxxxxxxx>]

On Thu, Dec 20, 2018 at 12:33 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
> >>> On 20.12.18 at 06:58, <christopher.w.clark@xxxxxxxxx> wrote:
> > On Wed, Dec 12, 2018 at 3:53 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >> >>> On 01.12.18 at 02:32, <christopher.w.clark@xxxxxxxxx> wrote:
> >> > +static struct argo_ring_info *
> >> > +argo_ring_find_info_by_match(const struct domain *d, uint32_t port,
> >> > +                             domid_t partner_id, uint64_t 
> >> > partner_cookie)
> >> > +{
> >> > +    argo_ring_id_t id;
> >> > +    struct argo_ring_info *ring_info;
> >> > +
> >> > +    ASSERT(rw_is_locked(&d->argo->lock));
> >> > +
> >> > +    id.addr.port = port;
> >> > +    id.addr.domain_id = d->domain_id;
> >> > +    id.partner = partner_id;
> >> > +
> >> > +    ring_info = argo_ring_find_info(d, &id);
> >> > +    if ( ring_info && (partner_cookie == ring_info->partner_cookie) )
> >> > +        return ring_info;
> >>
> >> Such a cookie makes mismatches unlikely, but it doesn't exclude
> >> them. If there are other checks, is the cookie useful at all?
> >
> > Yes, I think so and it's proved useful elsewhere in the second
> > version of the series: it helps avoid sending signals to incorrect
> > domains that may not be argo-enabled.
>
> "It helps avoid" still isn't "it allows to avoid", i.e. it still sounds like
> an approach reducing likelihood instead of one excluding mistakes
> altogether.

ok, I'm at the point where I'm close to having a version three of the
series to post that addresses all the feedback so far, plus some
additional improvements, with the following two items remaining to
discuss:

1) the domain_cookie, with Jan's question about a) its exclusion of
mismatches and b) its utility.

Given the expressed concern that the timer-based cookie initialization
does not necessarily exclude mismatches, I've reimplemented it as a
simple 128-bit counter protected by the L1 lock: this does now exclude
mismatches.

The utility of the cookie follows from this:

domid, despite its name, is not a unique domain identifier; it's a
temporally unique id: Xen will ensure that no two domains that execute
concurrently have the same domid. Domain authentication needs to take
this into account.

With Argo, it affects these points:

* ring registration: when the partner domain domid is specified, argo
finds the currently executing domain with that domid, and needs to
be able to confirm that it is the same domain later when a sendv is
issued.

* sendv: needs to confirm that the domain sending a message is the same
as the single domain authorized to transmit when the ring was first
registered.

* notify: the querying domain asks about free space, and if there's not
enough then a record is kept internal to the hypervisor, and a signal
will be sent to the caller later when sufficient space becomes
available.  Before sending the signal, Xen needs to confirm that the
current domain with the domid it remembered is the same as the one that
issued the query, otherwise Xen is sending spurious signals to domains
that are not expecting it (and unless it checks, may not even be
argo-enabled).

* domain teardown: in the absence of the domain cookie, or an
alternative data structure that achieves the same ability to
distinguish a reincarnated domain, all the rings that are registered
that authorize the dying domid to send need to be torn down with
suitable notification to their owners, and all the pending signals for
that domain about available free space need to be nullified, to prevent
a later domain inheriting these credentials and signals.

Doing so either entails a potentially-expensive walk of all rings of all
domains, plus all the pending notifications on all rings the domain can
access, or additional complexity with new data structures storing
further metadata on the authorized domain on ring registration, etc.
The domain cookie which enables identity confirmation on a domid is
a reasonable alternative solution.

So: if the switch to a simple counter is sufficient to mitigate the
mismatch concern, and the utility of the cookie is potentially
acceptable, I'll post a v3 series for review with that present.


2) the p2m type of the guest-supplied memory for the ring.

Roger raised a query about not pinning the p2m type of memory
used for the ring, and my response on 21st December is here:

https://lists.xenproject.org/archives/html/xen-devel/2018-12/msg02204.html

At the moment, I haven't changed that code. If the p2m type is changed
after ring registration, is it a problem? If not, then I think the code is
OK; but if so then a pointer to what makes it problematic would be
helpful to determine an appropriate next step.

thanks,

Christopher

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel