[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 21/22] libxc: range checks in xc_dom_p2m_host and _guest



At 16:58 +0100 on 11 Jun (1370969926), Ian Jackson wrote:
> Andrew Cooper writes ("Re: [Xen-devel] [PATCH 21/22] libxc: range checks in 
> xc_dom_p2m_host and _guest"):
> > On 07/06/2013 19:33, Andrew Cooper wrote:
> > > On 07/06/13 19:27, Ian Jackson wrote:
> > >> v6: Check for underflow too (thanks to Andrew Cooper).
> ...
> > >> diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
> > >> index 567913f..2e33ee7 100644
> > >> --- a/tools/libxc/xc_dom.h
> > >> +++ b/tools/libxc/xc_dom.h
> > >> @@ -341,6 +341,8 @@ static inline xen_pfn_t xc_dom_p2m_host(struct 
> > >> xc_dom_image *dom, xen_pfn_t pfn)
> > >>  {
> > >>      if (dom->shadow_enabled)
> > >>          return pfn;
> > 
> > The above should probably be
> > 
> > if (dom->shadow_enabled)
> >     return pfn < dom->total_pages ? pfn : INVALID_MFN;
> > 
> > So the dom->shadow_enable case also gets upper range checking.
> 
> Are you sure this shouldn't involve rambase_pfn, as the next test
> does ?  Here:
> 
> > >> +    if (pfn < dom->rambase_pfn || pfn >= dom->rambase_pfn + 
> > >> dom->total_pages)
> > >> +        return INVALID_MFN;
> 
> If it should then the right fix would be to move the check to before
> the shadow_enabled test.  (In both xc_dom_vaddr_to_ptr and
> xc_dom_p2m_guest.)
> 
> I'm not very familiar with the semantics of these functions.  I've
> CC'd Tim Deegan who can maybe help advise...

With the caveat that I'm not familiar with the PV builder code I'd say
the shadow case is fine without extra checks.  If the guest is really
paging_mode_translate then Xen will reject any invalid gfns when the
caller tries to map them.

Cheers,

Tim.

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.