Re: [Xen-devel] [PATCH v5 2/2] allow hardware domain != dom0

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 04/17/2014 03:24 AM, Jan Beulich wrote:
> > > > On 16.04.14 at 21:13, <dgdegra@xxxxxxxxxxxxx> wrote:
> > On 04/16/2014 03:06 PM, Andrew Cooper wrote:
> > > On 16/04/14 19:56, Daniel De Graaf wrote:
> > > >    static unsigned int __read_mostly extra_dom0_irqs = 256;
> > > >    static unsigned int __read_mostly extra_domU_irqs = 32;
> > > >    static void __init parse_extra_guest_irqs(const char *s)
> > > > @@ -192,7 +242,7 @@ custom_param("extra_guest_irqs", parse_extra_guest_irqs);
> > > >    struct domain *domain_create(
> > > >        domid_t domid, unsigned int domcr_flags, uint32_t ssidref)
> > > >    {
> > > > -    struct domain *d, **pd;
> > > > +    struct domain *d, **pd, *old_hwdom = NULL;
> > > >        enum { INIT_xsm = 1u<<0, INIT_watchdog = 1u<<1, INIT_rangeset = 1u<<2,
> > > >               INIT_evtchn = 1u<<3, INIT_gnttab = 1u<<4, INIT_arch = 1u<<5 };
> > > >        int err, init_status = 0;
> > > > @@ -237,10 +287,12 @@ struct domain *domain_create(
> > > >        else if ( domcr_flags & DOMCRF_pvh )
> > > >            d->guest_type = guest_type_pvh;
> > > >
> > > > -    if ( domid == 0 )
> > > > +    if ( domid == 0 || domid == hardware_domid )
> > > >        {
> > > > +        BUG_ON(domid >= DOMID_FIRST_RESERVED);
> > > Domid is a signed type.
> > >
> > > You also need ensure it is not negative, as assign_integer_param() from
> > > the command line parsing writes all values as unsigned.
> > While this is true, the domain ID has already been validated by the caller
> > of domain_create and so there is no need to check for domid < 0 here.  If
> > someone assigns an out-of-range domain ID to the hardware_domid field, the
> > system will act the same as if any other unused domain ID is specified: a
> > technically working but realistically unusable system.
> The thing is that you check the wrong entity: domid is always valid
> (as it is being picked by the caller) - you really want to BUG_ON() or
> panic() upon seeing hardware_domid >= DOMID_FIRST_RESERVED (and
> indeed I think in this case panic() would be the better option, as it
> gives a descriptive message for bad user input rather than a crash
> the reason for which one needs to look up in sources).
>
> As that's the only (non-cosmetic) change request, I'd be fine doing
> that change while committing, unless I hear to the contrary.
>
> Jan
That change is fine with me.


--
Daniel De Graaf
National Security Agency

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