Re: [PATCH] cgroup: Add new capability to allow a process to migrate other tasks between cgroups

[Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

Hello,

On Sat, Dec 17, 2016 at 10:06:51PM +0100, Mickaël Salaün wrote:
> If I understand correctly, this patch is intended to add a delegation
> feature to cgroup v1, which does not really make sense for the v2

It's more about upstreaming a workaround for android somewhat like
including binder into kernel.  It isn't adding actual cgroup
delegation to v1.  It's just splitting a small piece of CAP_SYS_ADMIN
to accomodate what android has been doing.

> because of the clean cgroup-v2 delegation design. However, this new
> capability impact both versions.

In the same way but it's not about cgroup delegation.  It's just
allowing splitting up CAP_SYS_ADMIN so that "no extra restrictions on
cgroup" can be given away in a safer way.

> However, even if a cgroup does not directly involve a limitation, it may
> be used to identify a group of processes for a security critical purpose
> (e.g. kill a group of process). It can then make sense to have a
> dedicated capability CAP_CGROUP to allow a process *without the right to
> write in cgroup.procs* to be allowed to move a process out of its
> current cgroup. This is similar to CAP_DAC_OVERRIDE but only for
> cgroup/controllers files (but not necessarily sufficient to modify all
> cgroups). This does not means that CAP_CGROUP should allow to move any
> process from any cgroup. The cgroup_procs_write_permission() should
> compose the checks for CAP_CGROUP and/or CAP_SYS_RESOURCE and/or
> CAP_SYS_ADMIN depending on the current use of the cgroup (i.e. cgroup
> controller, BPF program type, netfilter).

There's no reason to invent a whole new set of security policies for
cgroup.  It already got one which follows the filesystem permissions
with some extra restrictions.  The CAP split is purely to accomodate
android and that's it.  If that isn't good enough a reason, then
android should just keep carrying the patches it needs.  This doesn't
justify bolting on another permission model on cgroup in any way.

Thanks.

-- 
tejun