linux-kernel - Re: [PATCHv2 5/7] cgroup: introduce cgroup namespaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 31 Oct 2014 17:02:41 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Aditya Kali <adityakali@...gle.com>
Cc:	Tejun Heo <tj@...nel.org>, Li Zefan <lizefan@...wei.com>,
	Serge Hallyn <serge.hallyn@...ntu.com>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	cgroups@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linux API <linux-api@...r.kernel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Linux Containers <containers@...ts.linux-foundation.org>,
	Rohit Jnagal <jnagal@...gle.com>
Subject: Re: [PATCHv2 5/7] cgroup: introduce cgroup namespaces

On Fri, Oct 31, 2014 at 12:18 PM, Aditya Kali <adityakali@...gle.com> wrote:
> Introduce the ability to create new cgroup namespace. The newly created
> cgroup namespace remembers the cgroup of the process at the point
> of creation of the cgroup namespace (referred as cgroupns-root).
> The main purpose of cgroup namespace is to virtualize the contents
> of /proc/self/cgroup file. Processes inside a cgroup namespace
> are only able to see paths relative to their namespace root
> (unless they are moved outside of their cgroupns-root, at which point
>  they will see a relative path from their cgroupns-root).
> For a correctly setup container this enables container-tools
> (like libcontainer, lxc, lmctfy, etc.) to create completely virtualized
> containers without leaking system level cgroup hierarchy to the task.
> This patch only implements the 'unshare' part of the cgroupns.
>

> +       /* Prevent cgroup changes for this task. */
> +       threadgroup_lock(current);

This could just be me being dense, but what is the lock for?

> +
> +       /* CGROUPNS only virtualizes the cgroup path on the unified hierarchy.
> +        */
> +       cgrp = get_task_cgroup(current);
> +
> +       err = -ENOMEM;
> +       new_ns = alloc_cgroup_ns();
> +       if (!new_ns)
> +               goto err_out_unlock;
> +
> +       err = proc_alloc_inum(&new_ns->proc_inum);
> +       if (err)
> +               goto err_out_unlock;
> +
> +       new_ns->user_ns = get_user_ns(user_ns);
> +       new_ns->root_cgrp = cgrp;
> +
> +       threadgroup_unlock(current);
> +
> +       return new_ns;
> +
> +err_out_unlock:
> +       threadgroup_unlock(current);
> +err_out:
> +       if (cgrp)
> +               cgroup_put(cgrp);
> +       kfree(new_ns);
> +       return ERR_PTR(err);
> +}
> +
> +static int cgroupns_install(struct nsproxy *nsproxy, void *ns)
> +{
> +       pr_info("setns not supported for cgroup namespace");
> +       return -EINVAL;
> +}
> +
> +static void *cgroupns_get(struct task_struct *task)
> +{
> +       struct cgroup_namespace *ns = NULL;
> +       struct nsproxy *nsproxy;
> +
> +       rcu_read_lock();
> +       nsproxy = task->nsproxy;
> +       if (nsproxy) {
> +               ns = nsproxy->cgroup_ns;
> +               get_cgroup_ns(ns);
> +       }
> +       rcu_read_unlock();

How is this correct?  Other namespaces do it too, so it Must Be
Correct (tm), but I don't understand.  What is RCU protecting?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/