Re: [Xen-devel] [PATCH 03/22] xentoolcore, _restrict_all: Introduce new library and implementation

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Sep 19, 2017 at 11:47:12AM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH 03/22] xentoolcore, _restrict_all: Introduce new 
> library and implementation"):
> > On Fri, Sep 15, 2017 at 07:48:40PM +0100, Ian Jackson wrote:
> > > +int xentoolcore_restrict_all(uint32_t domid) {
> > > +    int r;
> > > +    Xentoolcore__Active_Handle *ah;
> > > +
> > > +    lock();
> > > +    XENTOOLCORE_LIST_FOREACH(ah, &handles, entry) {
> > > +        r = ah->restrict_callback(ah, domid);
> > 
> > Looking at the "Implement" patches for some libraries, I think we need
> > to stash domid in ah and filter base on that. If not, at least in the
> > case of duping /dev/null, we risk closing the handles we don't wish to
> > close.
> 
> I don't follow.
> 
> The libraries where we dup /dev/null do not support restriction and
> therefore the domid is irrelevant for them.
> 
> For the libraries where we call an actual restriction ioctl, the domid
> is recorded in the kernel.  The worst case is a bug where the restrict
> ioctl cannot be called more than once.  TBH if that is the case then
> we can just change the docs for xentoolcore_restrict_all to say that
> if you call it a 2nd time if may fail, even if given the same domid.

The impression I get from the name and parameter from this API is that
the following use case is allowed: a device model serving multiple
domains. The device model will open two sets of handlers of various
libraries. The device model will call restrict_all(domid) to restrict
its own privileges on certain domid when it sees fit.

Without filtering, the callbacks are called for all the domains at the
same time. The code as-is, when resctrict_all(dom1) is called, makes
privileges on dom2 are also dropped sometimes -- imagine a xenstore
callback registered for dom2 is called, which makes the connection
unusable for dom2.

If the aforementioned use case is not anticipated, I think we shouldn't
accept domid parameter for the resctrict_all function.

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