Re: [libvirt] [PATCH 1/2] util: share code between virExec and virCommandExec

virCommand is a version of virExec that doesn't fork, however it is just calling execve and doesn't honors setting uid/gid and pwd. This commit moves those pieces from virExec to virCommandExec and makes virExec use virCommandExec to avoid code duplication. --- src/util/vircommand.c | 92 ++++++++++++++++++++++++--------------------------- 1 file changed, 43 insertions(+), 49 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index e1bbc0526..aa97a5a10 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -481,21 +481,18 @@ virExec(virCommandPtr cmd) int childerr = -1; int tmpfd; char *binarystr = NULL; - const char *binary = NULL; int ret; struct sigaction waxon, waxoff; - gid_t *groups = NULL; - int ngroups; if (cmd->args[0][0] != '/') { - if (!(binary = binarystr = virFindFileInPath(cmd->args[0]))) { + if (!(binarystr = virFindFileInPath(cmd->args[0]))) { virReportSystemError(ENOENT, _("Cannot find '%s' in path"), cmd->args[0]); return -1; } - } else { - binary = cmd->args[0]; + VIR_FREE(cmd->args[0]); + cmd->args[0] = binarystr; } if (childin < 0) { @@ -556,9 +553,6 @@ virExec(virCommandPtr cmd) childerr = null; } - if ((ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups)) < 0) - goto cleanup; - pid = virFork(); if (pid < 0) @@ -577,9 +571,6 @@ virExec(virCommandPtr cmd) cmd->pid = pid; - VIR_FREE(binarystr); - VIR_FREE(groups); - return 0; } @@ -727,29 +718,6 @@ virExec(virCommandPtr cmd) } # endif - /* The steps above may need to do something privileged, so we delay - * setuid and clearing capabilities until the last minute. - */ - if (cmd->uid != (uid_t)-1 || cmd->gid != (gid_t)-1 || - cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS)) { - VIR_DEBUG("Setting child uid:gid to %d:%d with caps %llx", - (int)cmd->uid, (int)cmd->gid, cmd->capabilities); - if (virSetUIDGIDWithCaps(cmd->uid, cmd->gid, groups, ngroups, - cmd->capabilities, - !!(cmd->flags & VIR_EXEC_CLEAR_CAPS)) < 0) { - goto fork_error; - } - } - - if (cmd->pwd) { - VIR_DEBUG("Running child in %s", cmd->pwd); - if (chdir(cmd->pwd) < 0) { - virReportSystemError(errno, - _("Unable to change to %s"), cmd->pwd); - goto fork_error; - } - } - if (virCommandHandshakeChild(cmd) < 0) goto fork_error;

@@ -771,15 +739,10 @@ virExec(virCommandPtr cmd) /* Close logging again to ensure no FDs leak to child */ virLogReset();

- if (cmd->env) - execve(binary, cmd->args, cmd->env); - else - execv(binary, cmd->args); + if (virCommandExec(cmd) == -2) + goto fork_error; ret = errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE; - virReportSystemError(errno, - _("cannot execute binary %s"), - cmd->args[0]); fork_error: virDispatchError(NULL); @@ -789,9 +752,6 @@ virExec(virCommandPtr cmd) /* This is cleanup of parent process only - child should never jump here on error */ - VIR_FREE(groups); - VIR_FREE(binarystr); - /* NB we don't virReportError() on any failures here because the code which jumped here already raised an error condition which we must not overwrite */ @@ -2150,23 +2110,57 @@ virCommandProcessIO(virCommandPtr cmd) * in the hook after already forking / cloning, so does not attempt to * daemonize or preserve any FDs. * - * Returns -1 on any error executing the command. + * Returns -1 on any error executing the command, -2 if the error happen + * before running the command. + * * Will not return on success. */ #ifndef WIN32 int virCommandExec(virCommandPtr cmd) { + gid_t *groups = NULL; + int ngroups; + if (!cmd ||cmd->has_error == ENOMEM) { virReportOOMError(); - return -1; + return -2; } if (cmd->has_error) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid use of command API")); - return -1; + return -2; } - execve(cmd->args[0], cmd->args, cmd->env); + if ((ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups)) < 0) { + VIR_FREE(groups); + return -2; + } + + if (cmd->uid != (uid_t)-1 || cmd->gid != (gid_t)-1 || + cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS)) { + VIR_DEBUG("Setting child uid:gid to %d:%d with caps %llx", + (int)cmd->uid, (int)cmd->gid, cmd->capabilities); + if (virSetUIDGIDWithCaps(cmd->uid, cmd->gid, groups, ngroups, + cmd->capabilities, + !!(cmd->flags & VIR_EXEC_CLEAR_CAPS)) < 0) { + return -2; + } + } + VIR_FREE(groups); + + if (cmd->pwd) { + VIR_DEBUG("Running child in %s", cmd->pwd); + if (chdir(cmd->pwd) < 0) { + virReportSystemError(errno, + _("Unable to change to %s"), cmd->pwd); + return -2; + } + } + + if (cmd->env) + execve(cmd->args[0], cmd->args, cmd->env); + else + execv(cmd->args[0], cmd->args); virReportSystemError(errno, _("cannot execute binary %s"), -- 2.12.2 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list