From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH] libxc: x86: ensure that the initial mapping fits into the guest's memory Date: Mon, 07 Jan 2013 11:05:55 +0000 Message-ID: <50EABA2302000078000B357F@nat28.tlf.novell.com> References: <1357318360.14291.93.camel@zakaz.uk.xensource.com> <50EAB2F402000078000B352B@nat28.tlf.novell.com> <1357555056.14291.142.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1357555056.14291.142.camel@zakaz.uk.xensource.com> Content-Disposition: inline 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 Campbell Cc: Ian Jackson , Wei Liu , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org >>> On 07.01.13 at 11:37, Ian Campbell wrote: > On Mon, 2013-01-07 at 10:35 +0000, Jan Beulich wrote: >> >>> On 07.01.13 at 08:00, Jan Beulich wrote: >> >>>> Ian Campbell 01/04/13 5:53 PM >>> >> >>libxc: x86: ensure that the initial mapping fits into the guest's memory >> >> >> >>In particular we need to check that adding 512KB of slack and >> >>rounding up to a 4MB boundary do not overflow the guest's memory >> >>allocation. Otherwise we run off the end of the p2m when building the >> >>guest's initial page tables and populate them with garbage. >> > >> > Sadly our testing found this to cause SLE11 SP2 PV guests to not start >> > anymore (in its 4.1.x backported incarnation). I didn't get around yet to >> > check whether in the (apparently trivial) backport I overlooked something; >> > will do as soon as I get to the office. >> >> Switching the added panic invocation to >> >> xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY, >> "%s: not enough memory for initial mapping > (%#"PRIx64" > %#"PRIpfn")", >> __FUNCTION__, try_virt_end >> PAGE_SHIFT_X86, >> dom->total_pages); >> >> I see (with xend on 4.1.3) >> >> xc: error: panic: xc_dom_x86.c:100: count_pgtables: not enough memory for > initial mapping (0xffffffff81bff > 0x80000): Out of memory >> >> Did this really work for you? > > It did but I must confess I only tested with the mini-os test domain, > since that was what the initial bug was reported about and I stupidly > didn't think to test with a "real" kernel. > >> The 4.1.3 xl doesn't really want to work >> for me, so I can't directly cross check whether there's a behavioral >> difference between the two, but looking at an older log the virtual >> addresses reported for virt_alloc_end look similar. Afaict you need >> to subtract dom->parms.virt_base from try_virt_end. > > I bet virt_base == 0 for the mini-os kernel I tried. I'll respin and > retest. This is what works for me (also added printing of the relevant value, and dropping the unchanged parts of the patch): --- a/tools/libxc/xc_dom_x86.c +++ b/tools/libxc/xc_dom_x86.c @@ -82,6 +82,7 @@ static int count_pgtables(struct xc_dom_ { int pages, extra_pages; xen_vaddr_t try_virt_end; + xen_pfn_t try_pfn_end; extra_pages = dom->alloc_bootstack ? 1 : 0; extra_pages += dom->extra_pages; @@ -91,6 +92,16 @@ static int count_pgtables(struct xc_dom_ { try_virt_end = round_up(dom->virt_alloc_end + pages * PAGE_SIZE_X86, bits_to_mask(22)); /* 4MB alignment */ + try_pfn_end = (try_virt_end - dom->parms.virt_base) >> PAGE_SHIFT_X86; + + if ( try_pfn_end > dom->total_pages ) + { + xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY, + "%s: not enough memory for initial mapping (%#"PRIpfn" > %#"PRIpfn")", + __FUNCTION__, try_pfn_end, dom->total_pages); + return -ENOMEM; + } + dom->pg_l4 = nr_page_tables(dom, dom->parms.virt_base, try_virt_end, l4_bits); dom->pg_l3 = Jan