[Lguest] Error inserting lg: Cannot allocate memory

[Rusty Russell]

On Wednesday 05 March 2008 00:47:34 Fabio Checconi wrote:
> > From: Rusty Russell <rusty at rustcorp.com.au>
> >  	/* Now we reserve the "virtual memory area" we want: 0xFFC00000
> >  	 * (SWITCHER_ADDR).  We might not get it in theory, but in practice
> > -	 * it's worked so far. */
> > +	 * it's worked so far.  The +1 is because __get_vm_area allocates a
> > +	 * guard page, so we need space for that. */
> >  	switcher_vma = __get_vm_area(TOTAL_SWITCHER_PAGES * PAGE_SIZE,
> > -				       VM_ALLOC, SWITCHER_ADDR, VMALLOC_END);
> > +				     VM_ALLOC, SWITCHER_ADDR, SWITCHER_ADDR
> > +				     + (TOTAL_SWITCHER_PAGES+1) * PAGE_SIZE);
> >  	if (!switcher_vma) {
> >  		err = -ENOMEM;
> >  		printk("lguest: could not map switcher pages high\n");
>
> With the original .config, VMALLOC_END was lower than SWITCHER_ADDR,
> with this patch we may be forcing an allocation beyond VMALLOC_END,
> and this may cause the allocated region to fall in the fixmap/PKMAP
> area.

Hi Fabio,

   Yes, it's true we can allocate beyond VMALLOC_END, but that's OK: we're not
a vmalloc.  We currently will not be in the fixmap area by a long way, but
I've added a check, just in case.

   Also, the previous comment confused inflo: is this one better?

Thanks for the feedback!
Rusty.
==
Robert Bragg's 5dc331852848a38ca00a2817e5b98a1d0561b116 tightened
(ie. fixed) the checking in __get_vm_area, and it broke lguest.

lguest should pass the exact "end" it wants, not some random constant
(it was possible previously that it would actually get an address
different from SWITCHER_ADDR).

Also, Fabio Checconi pointed out that we should make sure we're not
hitting the fixmap area.

Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
Cc: Robert Bragg <robert at sixbynine.org>
---
 drivers/lguest/core.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff -r f5dc1442dc61 drivers/lguest/core.c
--- a/drivers/lguest/core.c	Tue Mar 04 14:21:38 2008 +1100
+++ b/drivers/lguest/core.c	Wed Mar 05 14:16:43 2008 +1100
@@ -69,11 +69,22 @@ static __init int map_switcher(void)
 		switcher_page[i] = virt_to_page(addr);
 	}
 
+	/* First we check that the Switcher won't overlap the fixmap area at
+	 * the top of memory.  It's currently nowhere near, but it could have
+	 * very strange effects if it ever happened. */
+	if (SWITCHER_ADDR + (TOTAL_SWITCHER_PAGES+1)*PAGE_SIZE > FIXADDR_START){
+		err = -ENOMEM;
+		printk("lguest: mapping switcher would thwack fixmap\n");
+		goto free_pages;
+	}
+
 	/* Now we reserve the "virtual memory area" we want: 0xFFC00000
 	 * (SWITCHER_ADDR).  We might not get it in theory, but in practice
-	 * it's worked so far. */
+	 * it's worked so far.  The end address needs +1 because __get_vm_area
+	 * allocates an extra guard page, so we need space for that. */
 	switcher_vma = __get_vm_area(TOTAL_SWITCHER_PAGES * PAGE_SIZE,
-				       VM_ALLOC, SWITCHER_ADDR, VMALLOC_END);
+				     VM_ALLOC, SWITCHER_ADDR, SWITCHER_ADDR
+				     + (TOTAL_SWITCHER_PAGES+1) * PAGE_SIZE);
 	if (!switcher_vma) {
 		err = -ENOMEM;
 		printk("lguest: could not map switcher pages high\n");