From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 21/22] libxc: range checks in xc_dom_p2m_host and _guest Date: Sat, 8 Jun 2013 13:05:07 +0100 Message-ID: <51B31DF3.7090201@citrix.com> References: <1370629642-6990-1-git-send-email-ian.jackson@eu.citrix.com> <1370629642-6990-22-git-send-email-ian.jackson@eu.citrix.com> <51B2278C.8030507@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <51B2278C.8030507@citrix.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: Ian Jackson Cc: "xen-devel@lists.xensource.com" , "mattjd@gmail.com" , "security@xen.org" List-Id: xen-devel@lists.xenproject.org On 07/06/2013 19:33, Andrew Cooper wrote: > On 07/06/13 19:27, Ian Jackson wrote: >> These functions take guest pfns and look them up in the p2m. They did >> no range checking. >> >> However, some callers, notably xc_dom_boot.c:setup_hypercall_page want >> to pass untrusted guest-supplied value(s). It is most convenient to >> detect this here and return INVALID_MFN. >> >> This is part of the fix to a security issue, XSA-55. >> >> Signed-off-by: Ian Jackson > Reviewed-by: Andrew Cooper On second thoughts, I retract this. > >> v6: Check for underflow too (thanks to Andrew Cooper). >> --- >> tools/libxc/xc_dom.h | 4 ++++ >> 1 files changed, 4 insertions(+), 0 deletions(-) >> >> 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. >> + if (pfn < dom->rambase_pfn || pfn >= dom->rambase_pfn + dom->total_pages) >> + return INVALID_MFN; >> return dom->p2m_host[pfn - dom->rambase_pfn]; >> } >> >> @@ -349,6 +351,8 @@ static inline xen_pfn_t xc_dom_p2m_guest(struct xc_dom_image *dom, >> { >> if (xc_dom_feature_translated(dom)) >> return pfn; Similarly here. ~Andrew >> + if (pfn < dom->rambase_pfn || pfn >= dom->rambase_pfn + dom->total_pages) >> + return INVALID_MFN; >> return dom->p2m_host[pfn - dom->rambase_pfn]; >> } >> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel