From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH v5 2/2] allow hardware domain != dom0 Date: Thu, 17 Apr 2014 10:19:04 -0400 Message-ID: <534FE2D8.8060603@tycho.nsa.gov> References: <1397674562-3444-1-git-send-email-dgdegra@tycho.nsa.gov> <1397674562-3444-2-git-send-email-dgdegra@tycho.nsa.gov> <534ED4D0.6070906@citrix.com> <534ED659.7030704@tycho.nsa.gov> <534F9DBD0200007800009DCD@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <534F9DBD0200007800009DCD@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Andrew Cooper Cc: Keir Fraser , Tim Deegan , Ian Jackson , Ian Campbell , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 04/17/2014 03:24 AM, Jan Beulich wrote: >>>> On 16.04.14 at 21:13, 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