cgroups/cgfsng: do not prematurely close file descriptors

When adding the new improved cgroup setup logic I didn't account for the fact
that we need the hierarchy fds up until chown. Add a dedicated cleanup method
to fix this:

lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, , 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, tasks, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )
lxc b1 20191212205052.712 WARN     cgfsng - cgroups/cgfsng.c:fchowmodat:1481 - Bad file descriptor - Failed to fchownat(-9, cgroup.procs, 1000000000, 0, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW )

Closes #3228.
Fixes: 1973b62aab41 ("cgroups/cgfsng: improve cgroup creation and removal")
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 83f41c005d6ee4c53119190826d2817579c62a9c..9751fb761201d53caea6ec77bca0121290c8e13c 100644 (file)
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1413,9 +1413,9 @@ __cgfsng_ops static bool cgfsng_monitor_enter(struct cgroup_ops *ops,
                        return log_error_errno(false, errno, "Failed to enter cgroup \"%s\"", h->monitor_full_path);
 
                /*
-                * We don't keep the fds for non-unified hierarchies around
+                * we don't keep the fds for non-unified hierarchies around
                 * mainly because we don't make use of them anymore after the
-                * core cgroup setup is done but also because they're quite a
+                * core cgroup setup is done but also because there are quite a
                 * lot of them.
                 */
                if (!is_unified_hierarchy(h))
@@ -1453,15 +1453,6 @@ __cgfsng_ops static bool cgfsng_payload_enter(struct cgroup_ops *ops,
                ret = lxc_writeat(h->cgfd_con, "cgroup.procs", pidstr, len);
                if (ret != 0)
                        return log_error_errno(false, errno, "Failed to enter cgroup \"%s\"", h->container_full_path);
-
-               /*
-                * We don't keep the fds for non-unified hierarchies around
-                * mainly because we don't make use of them anymore after the
-                * core cgroup setup is done but also because they're quite a
-                * lot of them.
-                */
-               if (!is_unified_hierarchy(h))
-                       close_prot_errno_disarm(h->cgfd_con);
        }
 
        return true;
@@ -1582,6 +1573,27 @@ __cgfsng_ops static bool cgfsng_chown(struct cgroup_ops *ops,
        return true;
 }
 
+__cgfsng_ops void cgfsng_payload_finalize(struct cgroup_ops *ops)
+{
+       if (!ops)
+               return;
+
+       if (!ops->hierarchies)
+               return;
+
+       for (int i = 0; ops->hierarchies[i]; i++) {
+               struct hierarchy *h = ops->hierarchies[i];
+               /*
+                * we don't keep the fds for non-unified hierarchies around
+                * mainly because we don't make use of them anymore after the
+                * core cgroup setup is done but also because there are quite a
+                * lot of them.
+                */
+               if (!is_unified_hierarchy(h))
+                       close_prot_errno_disarm(h->cgfd_con);
+       }
+}
+
 /* cgroup-full:* is done, no need to create subdirs */
 static bool cg_mount_needs_subdirs(int type)
 {
@@ -3253,6 +3265,7 @@ struct cgroup_ops *cgfsng_ops_init(struct lxc_conf *conf)
        cgfsng_ops->payload_delegate_controllers = cgfsng_payload_delegate_controllers;
        cgfsng_ops->payload_create = cgfsng_payload_create;
        cgfsng_ops->payload_enter = cgfsng_payload_enter;
+       cgfsng_ops->payload_finalize = cgfsng_payload_finalize;
        cgfsng_ops->escape = cgfsng_escape;
        cgfsng_ops->num_hierarchies = cgfsng_num_hierarchies;
        cgfsng_ops->get_hierarchies = cgfsng_get_hierarchies;

diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h
index ac7cbe5f328fb2a71199cd1ce6aaa2a03aaeacbd..46644c8445f4b618565615b7dad0acd5a2a8b1a4 100644 (file)
--- a/src/lxc/cgroups/cgroup.h
+++ b/src/lxc/cgroups/cgroup.h
@@ -166,6 +166,7 @@ struct cgroup_ops {
                                 struct lxc_handler *handler);
        bool (*monitor_delegate_controllers)(struct cgroup_ops *ops);
        bool (*payload_delegate_controllers)(struct cgroup_ops *ops);
+       void (*payload_finalize)(struct cgroup_ops *ops);
 };
 
 extern struct cgroup_ops *cgroup_init(struct lxc_conf *conf);

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 660187c0c5461b5a51f097e472d55d20a5e3e19f..d62acf4bd8a7924c108107d6fc9ff060aa05d326 100644 (file)
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -1922,6 +1922,9 @@ static int lxc_spawn(struct lxc_handler *handler)
                }
        }
 
+       cgroup_ops->payload_finalize(cgroup_ops);
+       TRACE("Finished setting up cgroups");
+
        /* Run any host-side start hooks */
        ret = run_lxc_hooks(name, "start-host", conf, NULL);
        if (ret < 0) {