Commit 5230ff0c authored by Philip Sanderson's avatar Philip Sanderson Committed by Rusty Russell

lguest: example launcher to use guard pages, drop PROT_EXEC, fix limit logic

PROT_EXEC seems to be completely unnecessary (as the lguest binary
never executes there), and will allow it to work with SELinux (and
more importantly, PaX :-) as they can/do forbid writable and
executable mappings.

Also, map PROT_NONE guard pages at start and end of guest memory for extra
paranoia.

I changed the length check to addr + size > guest_limit because >= is wrong
(addr of 0, size of getpagesize() with a guest_limit of getpagesize() would
false positive).
Signed-off-by: default avatarRusty Russell <rusty@rustcorp.com.au>
parent 8aeb36e8
...@@ -301,20 +301,27 @@ static void *map_zeroed_pages(unsigned int num) ...@@ -301,20 +301,27 @@ static void *map_zeroed_pages(unsigned int num)
/* /*
* We use a private mapping (ie. if we write to the page, it will be * We use a private mapping (ie. if we write to the page, it will be
* copied). * copied). We allocate an extra two pages PROT_NONE to act as guard
* pages against read/write attempts that exceed allocated space.
*/ */
addr = mmap(NULL, getpagesize() * num, addr = mmap(NULL, getpagesize() * (num+2),
PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE, fd, 0); PROT_NONE, MAP_PRIVATE, fd, 0);
if (addr == MAP_FAILED) if (addr == MAP_FAILED)
err(1, "Mmapping %u pages of /dev/zero", num); err(1, "Mmapping %u pages of /dev/zero", num);
if (mprotect(addr + getpagesize(), getpagesize() * num,
PROT_READ|PROT_WRITE) == -1)
err(1, "mprotect rw %u pages failed", num);
/* /*
* One neat mmap feature is that you can close the fd, and it * One neat mmap feature is that you can close the fd, and it
* stays mapped. * stays mapped.
*/ */
close(fd); close(fd);
return addr; /* Return address after PROT_NONE page */
return addr + getpagesize();
} }
/* Get some more pages for a device. */ /* Get some more pages for a device. */
...@@ -346,7 +353,7 @@ static void map_at(int fd, void *addr, unsigned long offset, unsigned long len) ...@@ -346,7 +353,7 @@ static void map_at(int fd, void *addr, unsigned long offset, unsigned long len)
* done to it. This allows us to share untouched memory between * done to it. This allows us to share untouched memory between
* Guests. * Guests.
*/ */
if (mmap(addr, len, PROT_READ|PROT_WRITE|PROT_EXEC, if (mmap(addr, len, PROT_READ|PROT_WRITE,
MAP_FIXED|MAP_PRIVATE, fd, offset) != MAP_FAILED) MAP_FIXED|MAP_PRIVATE, fd, offset) != MAP_FAILED)
return; return;
...@@ -576,10 +583,10 @@ static void *_check_pointer(unsigned long addr, unsigned int size, ...@@ -576,10 +583,10 @@ static void *_check_pointer(unsigned long addr, unsigned int size,
unsigned int line) unsigned int line)
{ {
/* /*
* We have to separately check addr and addr+size, because size could * Check if the requested address and size exceeds the allocated memory,
* be huge and addr + size might wrap around. * or addr + size wraps around.
*/ */
if (addr >= guest_limit || addr + size >= guest_limit) if ((addr + size) > guest_limit || (addr + size) < addr)
errx(1, "%s:%i: Invalid address %#lx", __FILE__, line, addr); errx(1, "%s:%i: Invalid address %#lx", __FILE__, line, addr);
/* /*
* We return a pointer for the caller's convenience, now we know it's * We return a pointer for the caller's convenience, now we know it's
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment