Re: [RFC PATCH] xen/memory: Introduce a hypercall to provide unallocated space

[Oleksandr <olekstysh@xxxxxxxxx>]

On 05.08.21 19:37, Daniel P. Smith wrote:
> Hey Oleksandr!
Hi Daniel.


> On 8/5/21 11:59 AM, Oleksandr wrote:
> > On 05.08.21 18:03, Daniel P. Smith wrote:
> >
> > Hi Daniel.
> >
> > > On 7/28/21 12:18 PM, Oleksandr Tyshchenko wrote:
> > > > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > > ...
> > >
> > > > diff --git a/xen/common/memory.c b/xen/common/memory.c
> > > > index e07bd9a..3f9b816 100644
> > > > --- a/xen/common/memory.c
> > > > +++ b/xen/common/memory.c
> > > > @@ -1811,6 +1811,62 @@ long do_memory_op(unsigned long cmd,
> > > > XEN_GUEST_HANDLE_PARAM(void) arg)
> > > >                start_extent);
> > > >            break;
> > > >    +    case XENMEM_get_unallocated_space:
> > > > +    {
> > > > +        struct xen_get_unallocated_space xgus;
> > > > +        struct xen_unallocated_region *regions;
> > > > +
> > > > +        if ( unlikely(start_extent) )
> > > > +            return -EINVAL;
> > > > +
> > > > +        if ( copy_from_guest(&xgus, arg, 1) ||
> > > > +             !guest_handle_okay(xgus.buffer, xgus.nr_regions) )
> > > > +            return -EFAULT;
> > > > +
> > > > +        d = rcu_lock_domain_by_any_id(xgus.domid);
> > > > +        if ( d == NULL )
> > > > +            return -ESRCH;
> > > > +
> > > > +        rc = xsm_get_unallocated_space(XSM_HOOK, d);
> > > Not sure if you are aware but XSM_HOOK is a no-op check, meaning that
> > > you are allowing any domain to do this operation on any other domain. In
> > > most cases there is an XSM check at the beginning of the hypercall
> > > processing to do an initial clamp down but I am pretty sure there is no
> > > prior XSM check on this path. Based on my understanding of how this is
> > > intended, which may be incorrect, but I think you would actually want
> > > XSM_TARGET.the
> > Thank you for pointing this out.
> > I am aware what the XSM_HOOK is, but I was thinking what the default
> > action would be better suited for that hypercall, and failed to think of
> > a better alternative.
> > I was going to choose XSM_TARGET, but the description "/* Can perform on
> > self or your target domain */" confused me a bit, as there was no target
> > domain involved as I thought, XSM_PRIV
> > sounded too strictly to me, etc. So, I decided to leave a "hook" for the
> > RFC version. But, now I see that XSM_TARGET might be indeed better
> > choice across all possible variants.
> If you unravel the craftiness that is xsm_default_action, there is
> actually a bit of hierarchy there. If you set the default_action to
> XSM_TARGET, it will first check if calling domain(src) is the target,
> then falls into the XSM_DM_PRIV check which is if src->target == target,
> and then finally checks if is_control_domain(src). That will constrict
> the operation so that a domain can call it on itself, a device model
> domain (stubdom) can call it on the domain it is backing, and the
> control domain can make the call. I am not a 100% sure on this but I do
> not believe a hardware domain would be able to make the call with it set
> to XSM_TARGET and not employing Flask. Hope this helps.
This definitely helps. Thank you for the explanation.


>
--
Regards,

Oleksandr Tyshchenko