Re: [Xen-devel] [PATCH] libxc: x86: ensure that the initial mapping fits into the guest's memory

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.01.13 at 11:37, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 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 <Ian.Campbell@xxxxxxxxxx> 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


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