> -----Original Message----- > From: Ian Jackson <ian.jack...@citrix.com> > Sent: 16 January 2020 19:36 > To: Durrant, Paul <pdurr...@amazon.co.uk> > Cc: xen-devel@lists.xenproject.org; Wei Liu <w...@xen.org>; Anthony Perard > <anthony.per...@citrix.com>; Andrew Cooper <andrew.coop...@citrix.com>; > George Dunlap <george.dun...@citrix.com>; Jan Beulich <jbeul...@suse.com>; > Julien Grall <jul...@xen.org>; Konrad Rzeszutek Wilk > <konrad.w...@oracle.com>; Stefano Stabellini <sstabell...@kernel.org>; > jandr...@gmail.com > Subject: Re: [PATCH v3 4/6] libxl: allow creation of domains with a > specified or random domid > > Hi. This broadly contains what I expected, but: > > Paul Durrant writes ("[PATCH v3 4/6] libxl: allow creation of domains with > a specified or random domid"): > > > + for (;;) { > > + if (info->domid == RANDOM_DOMID) { > > + uint16_t v; > > + > > + /* Randomize lower order bytes */ > > + ret = libxl__random_bytes(gc, (void *)&v, sizeof(v)); > > + if (ret < 0) > > + break; > > + > > + v &= DOMID_MASK; > > + if (!libxl_domid_valid_guest(v) || > > + libxl__is_retired_domid(gc, v)) > > + continue; > > + > > + *domid = v; > > + } > > + > > + ret = xc_domain_create(ctx->xch, domid, &create); > > + if (ret == 0 || errno != EEXIST || info->domid != > RANDOM_DOMID) > > + break; > > + } > > I think this has a race. > > Thread A, in domain destroy Thread B, in code above > > choose domid V > check V in recent domid list > > add V to recent domid list > destroy domain V in Xen > > create domain V in Xen > continue constructing V > > Thread B improperly constructs a new guest using V, exposing anyone > who was talking about V a moment ago to bugs. Some code might even > fail to spot the interval where V does not exist and carry on talking > to the new V as if it were the old one... > > I think there are only two possible solutions: > > - Check the domain's entry in the recent list *after* creating > the domain in Xen. This involves accepting that we will > reuse the domid but only for a domain we are in the early > stages of constructing, so hopefully without bad consequence? > > - Take the recent domid lock. >
Or take a global file lock in libxl around domain creation and destruction? > Also, it seems to me that we should check the recent domid list if we > let Xen choose the domid. Maybe that can be in a subsequent patch... > Well, we could solve all this, remove the need for a file and all the associated complexity by simply keeping history inside the hypervisor. I don't know how the Xen maintainers will feel about that though, as Xen itself shouldn't have a problem with eager domid re-use. Paul > Thanks, > Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel