Re: [Xen-devel] [PATCH v3 1/6] libxl: allow /local/domain/$LIBXL_TOOLSTACK_DOMID/device-model/$DOMID to be written by $DOMID

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 25 Jun 2015, Ian Campbell wrote:
> On Wed, 2015-06-10 at 11:09 +0100, Stefano Stabellini wrote:
> > The device model is going to restrict its xenstore connection to $DOMID
> > level.
> 
> Am I correct in concluding that only oxenstored supports XS_RESTRICT? I
> don't see it in C xenstored at all.

That is correct. C xenstored ignores it.


> >  Let qemu-xen access
> > /local/domain/$LIBXL_TOOLSTACK_DOMID/device-model/$DOMID, as it is
> > required by QEMU to read/write the physmap. It doesn't contain any
> > information the guest is not already fully aware of.
> 
> docs/misc/xenstore-paths.markdown should also be updated to reflect the
> changes here (and document any keys which are missing, since you seem to
> list more here than the doc contains).

Actually docs/misc/xenstore-paths.markdown already specifies
~/device-model/$DOMID/*, I don't think there is any need to go into the
possible sub-directories there.  But I'll add a note on the possible
privilege level change.


> > Add a maximum limit of physmap entries to save, so that the guest cannot
> > DOS the toolstack.
> 
> How will we cope when the limit needs to be increased in the future for
> some reason?

The current limit, 12, is far higher than the actual number of entries
that is usually just 1 (vga). The limit is not likely to increase unless
the devices being modelled actually change significantly. In that case
we'll have a way to detect it in libxl.

The reason why I feel comfortable having such an high limit compared to
the actual usage, is that it is only meant to prevent the guest from
filling up dom0's disk, as this information is saved to file. As
xenstore entries have a size limit themselves, we should be pretty safe.

 
> There is an interesting general issue here which is that we have
> XS_RESTRICT which changes a connection to be treated as having the
> permissions of a given target domain instead of the originating
> privileged domain.
> 
> This means that in order to reduce the privileges of one thing we have
> to increase the privilege of the guest itself (by granting access to
> those paths), which seems rather counter-intuitive.
> 
> What we really want is a new privilege type which is "read/write to
> connections which are _privileged_ over $domid, but not $domid itself"
> and for XS_RESTRICT to imply that.
> 
> Retrofitting something like that to xenstored would be tricky I suspect.
> 
> When the physmap stuff was added doing it via xenstore was convenient
> because we weren't concerning ourselves with this deprivileging. How
> that we are though perhaps we should think about whether this is still
> appropriate and consider using a QMP command to request the list instead
> for example.

That is true, however this doesn't mean we cannot also add a QMP command
in the future too, especially if we follow Wei's suggestion and we only
change the permissions if xs_restrict. We could also implemented a
different XS_RESTRICT. In other words, I don't think we have to come up
with the best possible solution right now, we just need to improve over
what we already have. I think that this is an improvement.


> > @@ -1698,6 +1700,9 @@ int libxl__toolstack_save(uint32_t domid, uint8_t 
> > **buf,
> >                  &num);
> >      count = num;
> >  
> > +    if (count > MAX_PHYSMAP_ENTRIES)
> > +        return -1;
> 
> Probably worth logging some sort of clue here.

OK

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel