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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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...

Ian.

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