From 17e7cb9937a63ed8f9bb0fb6ac7302758be76846 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Mon, 20 Sep 2010 14:02:06 +0100 Subject: [PATCH] Fix error launching libguestfs when euid != uid. When writing to a RHEV target, virt-v2v launches the libguestfs appliance with euid:egid = 36:36, which is required to write to an NFS target using root_squash. Since we changed to using a cached appliance, this causes an error on start up, as the cached files are owned by root, but the cache directory is owned by 36:36. The reason is that bash resets euid to uid and egid to gid so when febootstrap-supermin-helper is executed, it runs as root:root. The cache directory was created by libguestfs directly so it has the correct ownership. This patch fixes the issue by using explicit fork/exec instead of system (ie. not going via a shell) and by setting the real UID and GID to the effective UID and GID before execing. --- src/appliance.c | 133 +++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 108 insertions(+), 25 deletions(-) diff --git a/src/appliance.c b/src/appliance.c index 3c3279b..2c1d7c9 100644 --- a/src/appliance.c +++ b/src/appliance.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #ifdef HAVE_SYS_TYPES_H @@ -49,6 +50,7 @@ static int contains_ordinary_appliance (guestfs_h *g, const char *path, void *da static char *calculate_supermin_checksum (guestfs_h *g, const char *supermin_path); static int check_for_cached_appliance (guestfs_h *g, const char *supermin_path, const char *checksum, char **kernel, char **initrd, char **appliance); static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, const char *checksum, char **kernel, char **initrd, char **appliance); +static int run_supermin_helper (guestfs_h *g, const char *supermin_path, const char *cachedir, size_t cdlen); /* Locate or build the appliance. * @@ -332,36 +334,20 @@ build_supermin_appliance (guestfs_h *g, char cachedir[cdlen]; snprintf (cachedir, cdlen, "%s/%s", tmpdir, checksum); - /* Don't worry about this failing, because the command below will - * fail if the directory doesn't exist. Note the directory might - * already exist, eg. if a tmp cleaner has removed the existing - * appliance but not the directory itself. + /* Don't worry about this failing, because the + * febootstrap-supermin-helper command will fail if the directory + * doesn't exist. Note the directory might already exist, eg. if a + * tmp cleaner has removed the existing appliance but not the + * directory itself. */ (void) mkdir (cachedir, 0755); - /* Set a sensible umask in the subprocess, so kernel and initrd - * output files are world-readable (RHBZ#610880). - */ - size_t cmdlen = 2 * strlen (supermin_path) + 3 * (cdlen + 16) + 256; - char cmd[cmdlen]; - snprintf (cmd, cmdlen, - "umask 0022; " - "febootstrap-supermin-helper%s " - "-f ext2 " - "-k '%s/kmod.whitelist' " - "'%s/supermin.d' " - host_cpu " " - "%s/kernel %s/initrd %s/root", - g->verbose ? " --verbose" : "", - supermin_path, supermin_path, - cachedir, cachedir, cachedir); if (g->verbose) - guestfs___print_timestamped_message (g, "%s", cmd); - int r = system (cmd); - if (r == -1 || WEXITSTATUS (r) != 0) { - error (g, _("external command failed: %s"), cmd); + guestfs___print_timestamped_message (g, "run febootstrap-supermin-helper"); + + int r = run_supermin_helper (g, supermin_path, cachedir, cdlen); + if (r == -1) return -1; - } if (g->verbose) guestfs___print_timestamped_message (g, "finished building supermin appliance"); @@ -376,6 +362,103 @@ build_supermin_appliance (guestfs_h *g, return 0; } +/* Run febootstrap-supermin-helper and tell it to generate the + * appliance. Note that we have to do an explicit fork/exec here. + * 'system' goes via the shell, and on systems that have bash, bash + * has a misfeature where it resets the euid to uid which breaks + * virt-v2v. 'posix_spawn' was also considered but that doesn't allow + * us to reset the umask. + */ +static int +run_supermin_helper (guestfs_h *g, const char *supermin_path, + const char *cachedir, size_t cdlen) +{ + pid_t pid = fork (); + if (pid == -1) { + perrorf (g, "fork"); + return -1; + } + + if (pid > 0) { /* Parent. */ + int status; + if (waitpid (pid, &status, 0) == -1) { + perrorf (g, "waitpid"); + return -1; + } + if (!WIFEXITED (status) || WEXITSTATUS (status) != 0) { + error (g, _("external command failed, see earlier error messages")); + return -1; + } + return 0; + } + + /* Child. */ + + /* Set a sensible umask in the subprocess, so kernel and initrd + * output files are world-readable (RHBZ#610880). + */ + umask (0022); + + /* Set uid/gid in the child. This is a workaround for a misfeature + * in bash which breaks virt-v2v - see the comment at the top of + * this function. + */ + if (getuid () == 0) { + int egid = getegid (); + int euid = geteuid (); + + if (egid != 0 || euid != 0) { + if (seteuid (0) == -1) { + perror ("seteuid"); + _exit (EXIT_FAILURE); + } + + if (setgid (egid) == -1) { + perror ("setgid"); + _exit (EXIT_FAILURE); + } + + if (setuid (euid) == -1) { + perror ("setuid"); + _exit (EXIT_FAILURE); + } + } + } + + size_t pathlen = strlen (supermin_path); + + const char *argv[30]; + size_t i = 0; + + argv[i++] = "febootstrap-supermin-helper"; + if (g->verbose) + argv[i++] = "--verbose"; + argv[i++] = "-f"; + argv[i++] = "ext2"; + argv[i++] = "-k"; + char whitelist[pathlen + 32]; + snprintf (whitelist, pathlen + 32, "%s/kmod.whitelist", supermin_path); + argv[i++] = whitelist; + char supermin_d[pathlen + 32]; + snprintf (supermin_d, pathlen + 32, "%s/supermin.d", supermin_path); + argv[i++] = supermin_d; + argv[i++] = host_cpu; + char kernel[cdlen + 32]; + snprintf (kernel, cdlen + 32, "%s/kernel", cachedir); + argv[i++] = kernel; + char initrd[cdlen + 32]; + snprintf (initrd, cdlen + 32, "%s/initrd", cachedir); + argv[i++] = initrd; + char root[cdlen + 32]; + snprintf (root, cdlen + 32, "%s/root", cachedir); + argv[i++] = root; + argv[i++] = NULL; + + execvp ("febootstrap-supermin-helper", (char * const *) argv); + perror ("execvp"); + _exit (EXIT_FAILURE); +} + /* Search elements of g->path, returning the first path element which * matches the predicate function 'pred'. * -- 1.8.3.1