Date:	Tue, 21 Oct 2014 12:02:37 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Aditya Kali <adityakali@...gle.com>
Cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Linux API <linux-api@...r.kernel.org>,
	Linux Containers <containers@...ts.linux-foundation.org>,
	Serge Hallyn <serge.hallyn@...ntu.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Tejun Heo <tj@...nel.org>, cgroups@...r.kernel.org,
	Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCHv1 7/8] cgroup: cgroup namespace setns support

On Tue, Oct 21, 2014 at 11:49 AM, Aditya Kali <adityakali@...gle.com> wrote:
> On Mon, Oct 20, 2014 at 10:49 PM, Andy Lutomirski <luto@...capital.net> wrote:
>> On Mon, Oct 20, 2014 at 10:42 PM, Eric W. Biederman
>> <ebiederm@...ssion.com> wrote:
>>>
>>> I do wonder if we think of this as chcgrouproot if there is a simpler
>>> implementation.
>>
>> Could be.  I'll defer to Aditya for that one.
>>
>
> More than chcgrouproot, its probably closer to pivot_cgroup_root. In
> addition to restricting the process to a cgroup-root, new processes
> entering the container should also be implicitly contained within the
> cgroup-root of that container.

Why?  Concretely, why should this be in the kernel namespace code
instead of in userspace?

> Implementing pivot_cgroup_root would
> probably involve overloading mount-namespace to now understand cgroup
> filesystem too. I did attempt combining cgroupns-root with mntns
> earlier (not via a new syscall though), but came to the conclusion
> that its just simpler to have a separate cgroup namespace and get
> clear semantics. One of the issues was that implicitly changing cgroup
> on setns to mntns seemed like a huge undesirable side-effect.
>
> About pinning: I really feel that it should be OK to pin processes
> within cgroupns-root. I think thats one of the most important feature
> of cgroup-namespace since its most common usecase is to containerize
> un-trusted processes - processes that, for their entire lifetime, need
> to remain inside their container.

So don't let them out.  None of the other namespaces have this kind of
constraint:

 - If you're in a mntns, you can still use fds from outside.
 - If you're in a netns, you can still use sockets from outside the namespace.
 - If you're in an ipcns, you can still use ipc handles from outside.

etc.

> And with explicit permission from
> cgroup subsystem (something like cgroup.may_unshare as you had
> suggested previously), we can make sure that unprivileged processes
> cannot pin themselves. Also, maintaining this invariant (your current
> cgroup is always under your cgroupns-root) keeps the code and the
> semantics simple.

I actually think it makes the semantics more complex.  The less policy
you stick in the kernel, the easier it is to understand the impact of
that policy.

>
> If we ditch the pinning requirement and allow the containarized
> process to move outside of its cgroupns-root, we will have to address
> atleast the following:
> * what does its /proc/self/cgroup  (and /proc/<pid>/cgroup in general)
> look like? We might need to just not show anything in
> /proc/<pid>/cgroup in such case (for default hierarchy).

The process should see the cgroup path relative to its cgroup ns.
Whether this requires a new /proc mount or happens automatically is an
open question.  (I *hate* procfs for reasons like this.)

> * how should future setns() and unshare() by such process behave?

Open question.

> * 'mount -t cgroup cgroup <mnt>' by such a process will yield unexpected result

You could disallow that and instead require 'mount -t cgroup -o
cgrouproot=. cgroup mnt' where '.' will be resolved at mount time
relative to the caller's cgroupns.

> * container will not remain migratable

Why not?

> * added code complexity to handle above scenarios
>
> I understand that having process pinned to a cgroup hierarchy might
> seem inconvenient. But even today (without cgroup namespaces), moving
> a task from one cgroup to another can fail for reasons outside of
> control of the task attempting the move (even if its privileged). So
> the userspace should already handle this scenario. I feel its not
> worth to add complexity in the kernel for this.

--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/

Powered by blists - more mailing lists