Re: [PATCHv1 6/8] cgroup: restrict cgroup operations within task's cgroupns
From: Serge E. Hallyn <hidden>
Date: 2014-10-17 09:28:19
Also in:
cgroups, lkml
Possibly related (same subject, not in this thread)
- 2014-10-19 · Re: [PATCHv1 6/8] cgroup: restrict cgroup operations within task's cgroupns · Eric W. Biederman <hidden>
Quoting Aditya Kali (adityakali-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org):
Restrict following operations within the calling tasks: * cgroup_mkdir & cgroup_rmdir * cgroup_attach_task * writes to cgroup files outside of task's cgroupns-root Also, read of /proc/<pid>/cgroup file is now restricted only to tasks under same cgroupns-root. If a task tries to look at cgroup of another task outside of its cgroupns-root, then it won't be able to see anything for the default hierarchy. This is same as if the cgroups are not mounted. Signed-off-by: Aditya Kali <redacted>
So this is a bit different from some other namespaces - if I have an open fd to a file, then setns into a mntns where that file is not addressable, I can still use the file. I guess not allowing attach to a cgroup outside our ns is a good failsafe as we'll otherwise risk falling off a cliff in some code, but I'm not sure the cgroup_file_write/mkdir/rmdir restrictions are needed. (And really I can fchdir to a directory not in my ns, so the cgroup-attach restriction is any more justified). Still I'm not strictly opposed ot this, so Acked-by: Serge Hallyn <redacted> just wanted to point this out.
quoted hunk ↗ jump to hunk
--- kernel/cgroup.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-)diff --git a/kernel/cgroup.c b/kernel/cgroup.c index f8099b4..2fc0dfa 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c@@ -2318,6 +2318,12 @@ static int cgroup_attach_task(struct cgroup *dst_cgrp, struct task_struct *task; int ret; + /* Only allow changing cgroups accessible within task's cgroup + * namespace. i.e. 'dst_cgrp' should be a descendant of task's + * cgroupns->root_cgrp. */ + if (!cgroup_is_descendant(dst_cgrp, task_cgroupns_root(leader))) + return -EPERM; + /* look up all src csets */ down_read(&css_set_rwsem); rcu_read_lock();@@ -2882,6 +2888,10 @@ static ssize_t cgroup_file_write(struct kernfs_open_file *of, char *buf, struct cgroup_subsys_state *css; int ret; + /* Reject writes to cgroup files outside of task's cgroupns-root. */ + if (!cgroup_is_descendant(cgrp, task_cgroupns_root(current))) + return -EINVAL; + if (cft->write) return cft->write(of, buf, nbytes, off);@@ -4560,6 +4570,13 @@ static int cgroup_mkdir(struct kernfs_node *parent_kn, const char *name, parent = cgroup_kn_lock_live(parent_kn); if (!parent) return -ENODEV; + + /* Allow mkdir only within process's cgroup namespace root. */ + if (!cgroup_is_descendant(parent, task_cgroupns_root(current))) { + ret = -EPERM; + goto out_unlock; + } + root = parent->root; /* allocate the cgroup and its ID, 0 is reserved for the root */@@ -4822,6 +4839,13 @@ static int cgroup_rmdir(struct kernfs_node *kn) if (!cgrp) return 0; + /* Allow rmdir only within process's cgroup namespace root. + * The process can't delete its own root anyways. */ + if (!cgroup_is_descendant(cgrp, task_cgroupns_root(current))) { + cgroup_kn_unlock(kn); + return -EPERM; + } + ret = cgroup_destroy_locked(cgrp); cgroup_kn_unlock(kn);@@ -5051,6 +5075,15 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns, if (root == &cgrp_dfl_root && !cgrp_dfl_root_visible) continue; + cgrp = task_cgroup_from_root(tsk, root); + + /* The cgroup path on default hierarchy is shown only if it + * falls under current task's cgroupns-root. + */ + if (root == &cgrp_dfl_root && + !cgroup_is_descendant(cgrp, task_cgroupns_root(current))) + continue; + seq_printf(m, "%d:", root->hierarchy_id); for_each_subsys(ss, ssid) if (root->subsys_mask & (1 << ssid))@@ -5059,7 +5092,6 @@ int proc_cgroup_show(struct seq_file *m, struct pid_namespace *ns, seq_printf(m, "%sname=%s", count ? "," : "", root->name); seq_putc(m, ':'); - cgrp = task_cgroup_from_root(tsk, root); path = cgroup_path(cgrp, buf, PATH_MAX); if (!path) { retval = -ENAMETOOLONG;-- 2.1.0.rc2.206.gedb03e5 _______________________________________________ Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org https://lists.linuxfoundation.org/mailman/listinfo/containers