[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] lxc: hoist supplemental group detection before clone



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 redhat com>
---
 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]