Re: [Xen-devel] [PATCH] docs/qemu-deprivilege: Revise and update with status and future plans

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 03/28/2018 01:47 PM, Ross Lagerwall wrote:
> On 03/27/2018 02:33 PM, Ian Jackson wrote:
>> George Dunlap writes ("Re: [PATCH] docs/qemu-deprivilege: Revise and
>> update with status and future plans"):
>>> Actually I think most of the user-facing stuff already in xl.cfg is
>>> inappropriate for that man page.  It might make sense to have a separate
>>> man page for it.
>>
>> I wouldn't object to that.
>>
>>> On 03/26/2018 05:43 PM, Ian Jackson wrote:
>>>> No.  Firstly, in each case, all relevant descriptors are restricted.
>>>> This is the purpose of the xentoolcore__restrict_* stuff.  Secondly,
>>>> xenstore *is* covered - but the xs fd is squashed so as to be totally
>>>> unuseable: xs.c uses xentoolcore__restrict_by_dup2_null.
>>>
>>> Ross already gave me some corrections on this; here is what I have:
>>>
>>> 8<---
>>> '''Description''': Close and restrict Xen-related file descriptors.
>>> Specifically:
>>>   * Close all xenstore-related file descriptors
>>>   * Make sure that extraneous `privcmd` and `evtchn` instances are
>>> closed
>>>   * Make sure that all open instances of `privcmd` and `evtchn` file
>>> descriptors have had IOCTL_PRIVCMD_RESTRICT and
>>> IOCTL_EVTCHN_RESTRICT_DOMID ioctls called on them, respectively.
>>> --->8
>>>
>>> It sounds like the last may be inaccurate for libxl?
>>
>> I don't think anything closes any extraneous fds.  My approach in
>> the libxc layer was to register all fds and have the restrict call
>> iterate over all of them.  So, I guess, drop your 2nd bullet.
>>
>> All of this is done by qemu calling xentoolcore_restrict_all, not by
>> libxl.
>>
>> Maybe the "extraneous privcmd and evtchn fds" Ross means are ones
>> inherited by qemu from the toolstack.
> 
> IIRC I didn't mention anything about extraneous fds. I'm not sure where
> that part came from

That was me regurgitating from memory something IanJ said, about QEMU
having loads of dangling fds lying around that he needed to figure out
how to check to make sure they were all closed.  Probably I misunderstood.

> Maybe this part shouldn't contain so much detail. Perhaps it could just
> refer to the documentation for xentoolcore_restrict_all?

Which documentation is this?  The description in
tools/libs/toolcore/include/xentoolcore.h describes the *intended
effect*, not the actual mechanism.  (It doesn't mention dup()'ing the
files to /dev/null, for instance.)

I do think it's important to have one place where someone can find all
the technical details of the restrictions made, so that they can verify
that restriction is sufficient for their needs (or determine that we've
missed something and let us know).

I'm happy to have a summary of what xentoolcore_restrict_all() does with
a reference to a more complete description elsewhere, if (or when) such
a description exists.

 -George

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