n Thu, Jan 31, 2019 at 8:19 AM Roger Pau Monné <roger....@citrix.com> wrote:
>
> On Wed, Jan 30, 2019 at 08:28:12PM -0800, Christopher Clark wrote:
> > The register op is used by a domain to register a region of memory for
> > receiving messages from either a specified other domain, or, if specifying a
> > wildcard, any domain.
> >
> > This operation creates a mapping within Xen's private address space that
> > will remain resident for the lifetime of the ring. In subsequent commits,
> > the hypervisor will use this mapping to copy data from a sending domain into
> > this registered ring, making it accessible to the domain that registered the
> > ring to receive data.
> >
> > Wildcard any-sender rings are default disabled and registration will be
> > refused with EPERM unless they have been specifically enabled with the
> > new mac-permissive flag that is added to the argo boot option here. The
> > reason why the default for wildcard rings is 'deny' is that there is
> > currently no means to protect the ring from DoS by a noisy domain
> > spamming the ring, affecting other domains ability to send to it. This
> > will be addressed with XSM policy controls in subsequent work.
> >
> > Since denying access to any-sender rings is a significant functional
> > constraint, the new option "mac-permissive" for the argo bootparam
> > enables overriding this. eg: "argo=1,mac-permissive=1"
> >
> > The p2m type of the memory supplied by the guest for the ring must be
> > p2m_ram_rw and the memory will be pinned as PGT_writable_page while the ring
> > is registered.
> >
> > This hypercall op and its interface currently only supports 4K-sized pages.
> >
> > Signed-off-by: Christopher Clark <christopher.cla...@baesystems.com>
> > Tested-by: Chris Patterson <patters...@ainfosec.com>
>
> Just one style issue below that should be fixed before commit, and two
> comments:
>
> Reviewed-by: Roger Pau Monné <roger....@citrix.com>
Thanks
> > +static int
> > +ring_map_page(const struct domain *d, struct argo_ring_info *ring_info,
> > + unsigned int i, void **out_ptr)
> > +{
> > + ASSERT(LOCKING_L3(d, ring_info));
> > +
> > + /*
> > + * FIXME: Investigate using vmap to create a single contiguous virtual
> > + * address space mapping of the ring instead of using the array of
> > single
> > + * page mappings.
> > + * Affects logic in memcpy_to_guest_ring, the mfn_mapping array data
> > + * structure, and places where ring mappings are added or removed.
> > + */
> > +
> > + if ( i >= ring_info->nmfns )
> > + {
> > + gprintk(XENLOG_ERR,
> > + "argo: ring (vm%u:%x vm%u) %p attempted to map page %u of
> > %u\n",
> > + ring_info->id.domain_id, ring_info->id.aport,
> > + ring_info->id.partner_id, ring_info, i, ring_info->nmfns);
> > + return -ENOMEM;
> > + }
> > + i = array_index_nospec(i, ring_info->nmfns);
> > +
> > + if ( !ring_info->mfns || !ring_info->mfn_mapping)
> ^ missing space
ack
> [...]
> > +static int
> > +copy_gfn_from_handle(XEN_GUEST_HANDLE_PARAM(void) gfn_hnd, bool compat,
> > + unsigned int i, gfn_t *out_gfn)
> > +{
> > + int ret;
> > +
> > +#ifdef CONFIG_COMPAT
> > + if ( compat )
> > + {
> > + XEN_GUEST_HANDLE_PARAM(compat_pfn_t) c_gfn_hnd =
> > + guest_handle_cast(gfn_hnd, compat_pfn_t);
> > + compat_pfn_t c_gfn;
> > +
> > + ret = __copy_from_guest_offset(&c_gfn, c_gfn_hnd, i, 1) ? -EFAULT
> > : 0;
> > + *out_gfn = _gfn(c_gfn);
> > + }
> > + else
>
> AFAICT you could place the #endif here and avoid the one below.
ack, thanks.
> > @@ -579,7 +1105,6 @@ compat_argo_op(unsigned int cmd,
> > XEN_GUEST_HANDLE_PARAM(void) arg1,
> > argo_dprintk("<-compat_argo_op(%u)=%ld\n", cmd, rc);
> >
> > return rc;
> > -
>
> This looks like a stray fix that should have gone into a different
> patch.
ack, fixed.
Christopher
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
