[PATCH] lxc: hoist supplemental group detection before clone

[Eric Blake <eblake@xxxxxxxxxx>]

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@xxxxxxxxxx>
---
 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list