[Lguest] Patches...
Philip Sanderson
philip.k.sanderson at gmail.com
Thu Nov 11 23:04:14 EST 2010
Patches look good; except it's missing the initgroups() / setresgid() /
setresuid() code.
I wasn't sure how gmail would handle pasting code into it.
On Thu, Nov 11, 2010 at 2:02 PM, Rusty Russell <rusty at rustcorp.com.au>wrote:
> Hi Philip,
>
> There was some whitespace damage so I ended up reworking and splitting
> your patches. Mainly flipped it to kernel-style coding, and added some
> commentry (some extracted from our email conversation).
>
> Here they are, for your ack/Signed-off-by.
>
> Thanks!
> Rusty.
>
> Subject: lguest: example launcher to use guard pages, drop PROT_EXEC, fix
> limit logic
> From: Philip Sanderson <philip.k.sanderson at gmail.com>
>
> 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: Rusty Russell <rusty at rustcorp.com.au>
>
> diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c
> --- a/Documentation/lguest/lguest.c
> +++ b/Documentation/lguest/lguest.c
> @@ -298,10 +298,12 @@ static void *map_zeroed_pages(unsigned i
>
> /*
> * 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 invalid read/write attempts that exceed allocated
> space.
> */
> - addr = mmap(NULL, getpagesize() * num,
> - PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE, fd, 0);
> + addr = mmap(NULL, getpagesize() * (num+2),
> + PROT_NONE, MAP_PRIVATE, fd, 0);
> +
> if (addr == MAP_FAILED)
> err(1, "Mmapping %u pages of /dev/zero", num);
>
> @@ -343,7 +345,7 @@ static void map_at(int fd, void *addr, u
> * done to it. This allows us to share untouched memory between
> * 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)
> return;
>
> @@ -573,10 +575,10 @@ static void *_check_pointer(unsigned lon
> unsigned int line)
> {
> /*
> - * We have to separately check addr and addr+size, because size
> could
> - * be huge and addr + size might wrap around.
> + * Check if the requested address and size exceeds the allocated
> memory,
> + * 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);
> /*
> * We return a pointer for the caller's convenience, now we know
> it's
>
>
> Subject: lguest: --username and --chroot options
> From: Philip Sanderson <philip.k.sanderson at gmail.com>
>
> I've attached a patch which implements dropping to privileges
> and chrooting to a directory.
>
> Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
>
> diff -ur linux-2.6.35.8-orig/Documentation/lguest/lguest.c
> linux-2.6.35.8/Documentation/lguest/lguest.c
>
> diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c
> --- a/Documentation/lguest/lguest.c
> +++ b/Documentation/lguest/lguest.c
> @@ -39,6 +39,9 @@
> #include <limits.h>
> #include <stddef.h>
> #include <signal.h>
> +#include <pwd.h>
> +#include <grp.h>
> +
> #include <linux/virtio_config.h>
> #include <linux/virtio_net.h>
> #include <linux/virtio_blk.h>
> @@ -1874,6 +1877,8 @@ static struct option opts[] = {
> { "block", 1, NULL, 'b' },
> { "rng", 0, NULL, 'r' },
> { "initrd", 1, NULL, 'i' },
> + { "username", 1, NULL, 'u' },
> + { "chroot", 1, NULL, 'c' },
> { NULL },
> };
> static void usage(void)
> @@ -1896,6 +1901,12 @@ int main(int argc, char *argv[])
> /* If they specify an initrd file to load. */
> const char *initrd_name = NULL;
>
> + /* Password structure for initgroups/setres[gu]id */
> + struct passwd *user_details = NULL;
> +
> + /* Directory to chroot to */
> + char *chroot_path = NULL;
> +
> /* Save the args: we "reboot" by execing ourselves again. */
> main_args = argv;
>
> @@ -1952,6 +1963,14 @@ int main(int argc, char *argv[])
> case 'i':
> initrd_name = optarg;
> break;
> + case 'u':
> + user_details = getpwnam(optarg);
> + if (!user_details)
> + err(1, "getpwnam failed, incorrect
> username?");
> + break;
> + case 'c':
> + chroot_path = optarg;
> + break;
> default:
> warnx("Unknown argument %s", argv[optind]);
> usage();
>
>
> Subject: lguest: document --rng in example Launcher
> From: Philip Sanderson <philip.k.sanderson at gmail.com>
>
> Rusty Russell wrote:
> > Ah, it will appear as /dev/hwrng. It's a weirdness of Linux that our
> actual
> > hardware number generators are not wired up to /dev/random...
>
> Reflected this in the documentation, thanks :-)
>
> Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
>
> diff --git a/Documentation/lguest/lguest.txt
> b/Documentation/lguest/lguest.txt
> --- a/Documentation/lguest/lguest.txt
> +++ b/Documentation/lguest/lguest.txt
> @@ -114,6 +114,11 @@ Running Lguest:
> See http://linux-net.osdl.org/index.php/Bridge for general information
> on how to get bridging working.
>
> +- Random number generation. Using the --rng option will provide a
> + /dev/hwrng in the guest that will read from the host's /dev/random.
> + Use this option in conjunction with rng-tools (see ../hw_random.txt)
> + to provide entropy to the guest kernel's /dev/random.
> +
> There is a helpful mailing list at
> http://ozlabs.org/mailman/listinfo/lguest
>
> Good luck!
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/lguest/attachments/20101111/20c2a2d7/attachment.html>
More information about the Lguest
mailing list