Commit 75c1256 states that virGetGroupList must not be called between fork and exec, then commit ee777e99 promptly violated that for lxc. Hoist the group detection to occur before clone. * src/lxc/lxc_container.c (__lxc_child_argv): Add members. (lxcContainerSetID): Adjust signature. (lxcContainerChild, lxcContainerStart): Adjust callers. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/lxc/lxc_container.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 940233b..50aaa36 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -106,6 +106,8 @@ struct __lxc_child_argv { char **ttyPaths; size_t nttyPaths; int handshakefd; + gid_t *groups; + int ngroups; }; static int lxcContainerMountFSBlock(virDomainFSDefPtr fs, @@ -349,26 +351,20 @@ int lxcContainerWaitForContinue(int control) * * Returns 0 on success or -1 in case of error */ -static int lxcContainerSetID(virDomainDefPtr def) +static int +lxcContainerSetID(virDomainDefPtr def, gid_t* groups, int ngroups) { - gid_t *groups; - int ngroups; - /* Only call virSetUIDGID when user namespace is enabled * for this container. And user namespace is only enabled * when nuidmap&ngidmap is not zero */ VIR_DEBUG("Set UID/GID to 0/0"); - if (def->idmap.nuidmap && - ((ngroups = virGetGroupList(0, 0, &groups) < 0) || - virSetUIDGID(0, 0, groups, ngroups) < 0)) { + if (def->idmap.nuidmap && virSetUIDGID(0, 0, groups, ngroups) < 0) { virReportSystemError(errno, "%s", _("setuid or setgid failed")); - VIR_FREE(groups); return -1; } - VIR_FREE(groups); return 0; } @@ -1981,7 +1977,7 @@ static int lxcContainerChild(void *data) cmd = lxcContainerBuildInitCmd(vmDef); virCommandWriteArgLog(cmd, 1); - if (lxcContainerSetID(vmDef) < 0) + if (lxcContainerSetID(vmDef, argv->groups, argv->ngroups) < 0) goto cleanup; root = virDomainGetRootFilesystem(vmDef); @@ -2054,6 +2050,7 @@ cleanup: VIR_FORCE_CLOSE(ttyfd); VIR_FORCE_CLOSE(argv->monitor); VIR_FORCE_CLOSE(argv->handshakefd); + VIR_FREE(argv->groups); if (ret == 0) { /* this function will only return if an error occurred */ @@ -2140,13 +2137,18 @@ int lxcContainerStart(virDomainDefPtr def, char *stack, *stacktop; lxc_child_argv_t args = { def, securityDriver, nveths, veths, control, - ttyPaths, nttyPaths, handshakefd}; + ttyPaths, nttyPaths, handshakefd, NULL, 0 }; /* allocate a stack for the container */ if (VIR_ALLOC_N(stack, stacksize) < 0) return -1; stacktop = stack + stacksize; + if ((args.ngroups = virGetGroupList(0, 0, &args.groups)) < 0) { + VIR_FREE(stack); + return -1; + } + cflags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD; if (userns_required(def)) { @@ -2157,6 +2159,7 @@ int lxcContainerStart(virDomainDefPtr def, virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Kernel doesn't support user namespace")); VIR_FREE(stack); + VIR_FREE(args.groups); return -1; } } @@ -2168,6 +2171,7 @@ int lxcContainerStart(virDomainDefPtr def, pid = clone(lxcContainerChild, stacktop, cflags, &args); VIR_FREE(stack); + VIR_FREE(args.groups); VIR_DEBUG("clone() completed, new container PID is %d", pid); if (pid < 0) { -- 1.8.1.4