fix cgfs failure for unpriv users

Cgmanager was taught awhile ago that only some cgroup controllers are
crucial.  Teach cgfs the same thing.

This patch needs improvement, but will fix failure of lxc without cgmanager
for unprivileged users for now.  In particular, needed improvements include:

1. the check for crucial subsystems needs to include lxc.use
2. we should keep a list of the actually used subsystems so we don't keep
trying to chmod and enter after create has found we couldn't use a particular
subsystem

This fixes unprivileged lxc use.  It does not appear to suffice to fix
nested unprivilegd lxd usage.

Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>

diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
index e2d8934655a018d07a9ee7c32d90af4f6990ec8c..8c1cb64eaef5a8baff0fdbdab0e99900d7ba630e 100644 (file)
--- a/src/lxc/cgfs.c
+++ b/src/lxc/cgfs.c
@@ -807,6 +807,17 @@ static char *cgroup_rename_nsgroup(const char *mountpath, const char *oldname, p
        return newname;
 }
 
+static bool is_crucial_hierarchy(struct cgroup_hierarchy *h)
+{
+       char **p;
+
+       for (p = h->subsystems; *p; p++) {
+               if (is_crucial_cgroup_subsystem(*p))
+                       return true;
+       }
+       return false;
+}
+
 /* create a new cgroup */
 static struct cgroup_process_info *lxc_cgroupfs_create(const char *name, const char *path_pattern, struct cgroup_meta_data *meta_data, const char *sub_pattern)
 {
@@ -974,8 +985,11 @@ static struct cgroup_process_info *lxc_cgroupfs_create(const char *name, const c
                                current_entire_path = NULL;
                                goto cleanup_name_on_this_level;
                        } else if (r < 0 && errno != EEXIST) {
-                               SYSERROR("Could not create cgroup '%s' in '%s'.", current_entire_path, info_ptr->designated_mount_point->mount_point);
-                               goto cleanup_from_error;
+                               if (is_crucial_hierarchy(info_ptr->hierarchy)) {
+                                       SYSERROR("Could not create cgroup '%s' in '%s'.", current_entire_path, info_ptr->designated_mount_point->mount_point);
+                                       goto cleanup_from_error;
+                               }
+                               goto skip;
                        } else if (r == 0) {
                                /* successfully created */
                                r = lxc_grow_array((void ***)&info_ptr->created_paths, &info_ptr->created_paths_capacity, info_ptr->created_paths_count + 1, 8);
@@ -999,6 +1013,7 @@ static struct cgroup_process_info *lxc_cgroupfs_create(const char *name, const c
                                        goto cleanup_from_error;
                                }
 
+skip:
                                /* already existed but path component of pattern didn't contain '%n',
                                 * so this is not an error; but then we don't need current_entire_path
                                 * anymore...
@@ -1180,7 +1195,7 @@ static int lxc_cgroupfs_enter(struct cgroup_process_info *info, pid_t pid, bool
 
                r = lxc_write_to_file(cgroup_tasks_fn, pid_buf, strlen(pid_buf), false);
                free(cgroup_tasks_fn);
-               if (r < 0) {
+               if (r < 0 && is_crucial_hierarchy(info_ptr->hierarchy)) {
                        SYSERROR("Could not add pid %lu to cgroup %s: internal error", (unsigned long)pid, cgroup_path);
                        return -1;
                }
@@ -1509,7 +1524,7 @@ static bool cgroupfs_mount_cgroup(void *hdata, const char *root, int type)
                        if (!abs_path)
                                goto out_error;
                        r = mount(abs_path, abs_path2, "none", MS_BIND, 0);
-                       if (r < 0) {
+                       if (r < 0 && is_crucial_hierarchy(info->hierarchy)) {
                                SYSERROR("error bind-mounting %s to %s", abs_path, abs_path2);
                                goto out_error;
                        }
@@ -2600,7 +2615,7 @@ static bool cgfs_chown(void *hdata, struct lxc_conf *conf)
                        continue;
                }
                r = do_cgfs_chown(cgpath, conf);
-               if (!r) {
+               if (!r && is_crucial_hierarchy(info_ptr->hierarchy)) {
                        ERROR("Failed chowning %s\n", cgpath);
                        free(cgpath);
                        return false;

diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
index 9f83a6e639ab389049c1e5d24c8c1221b94c6a60..937c36ed173048343dfa392a5bb8844597681c89 100644 (file)
--- a/src/lxc/cgmanager.c
+++ b/src/lxc/cgmanager.c
@@ -1242,21 +1242,6 @@ static bool subsys_is_writeable(const char *controller, const char *probe)
        return ret;
 }
 
-/*
- * Return true if this is a subsystem which we cannot do
- * without
- */
-static bool is_crucial_subsys(const char *s)
-{
-       if (strcmp(s, "systemd") == 0)
-               return true;
-       if (strcmp(s, "name=systemd") == 0)
-               return true;
-       if (strcmp(s, "freezer") == 0)
-               return true;
-       return false;
-}
-
 static char *get_last_controller_in_list(char *list)
 {
        char *p;
@@ -1302,7 +1287,7 @@ static bool verify_final_subsystems(const char *cgroup_use)
                char *p = get_last_controller_in_list(subsystems[i]);
 
                if (!subsys_is_writeable(p, probe)) {
-                       if (is_crucial_subsys(p)) {
+                       if (is_crucial_cgroup_subsystem(p)) {
                                ERROR("Cannot write to crucial subsystem %s\n",
                                        subsystems[i]);
                                goto out;

diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 8954733c12ca6e2a423dff97d6f5c411fd2174e7..8ee0629a1645fca69f1ca5241da5fb7bc5388dd4 100644 (file)
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -220,3 +220,18 @@ void prune_init_scope(char *cg)
                        *point = '\0';
        }
 }
+
+/*
+ * Return true if this is a subsystem which we cannot do
+ * without
+ */
+bool is_crucial_cgroup_subsystem(const char *s)
+{
+       if (strcmp(s, "systemd") == 0)
+               return true;
+       if (strcmp(s, "name=systemd") == 0)
+               return true;
+       if (strcmp(s, "freezer") == 0)
+               return true;
+       return false;
+}

diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
index e3d3ce4cc7464b1a7d6ee58ddd4cae16d8aebc97..63dcc925412441cabfac2a6a2a9641b91c1649c5 100644 (file)
--- a/src/lxc/cgroup.h
+++ b/src/lxc/cgroup.h
@@ -83,5 +83,6 @@ extern void cgroup_disconnect(void);
 extern cgroup_driver_t cgroup_driver(void);
 
 extern void prune_init_scope(char *cg);
+extern bool is_crucial_cgroup_subsystem(const char *s);
 
 #endif