7:26 a.m.
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