From: Johannes Weiner <hannes@cmpxchg.org>
To: Tejun Heo <tj@kernel.org>
Cc: lizefan@huawei.com, cgroups@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH v2 3/4] cgroup: require write perm on common ancestor when moving processes on the default hierarchy
Date: Thu, 18 Jun 2015 15:04:25 -0400 [thread overview]
Message-ID: <20150618190425.GB2182@cmpxchg.org> (raw)
In-Reply-To: <20150618175927.GD12934@mtj.duckdns.org>
On Thu, Jun 18, 2015 at 01:59:27PM -0400, Tejun Heo wrote:
> On traditional hierarchies, if a task has write access to "tasks" or
> "cgroup.procs" file of a cgroup and its euid agrees with the target,
> it can move the target to the cgroup; however, consider the following
> scenario. The owner of each cgroup is in the parentheses.
>
> R (root) - 0 (root) - 00 (user1) - 000 (user1)
> | \ 001 (user1)
> \ 1 (root) - 10 (user1)
>
> The subtrees of 00 and 10 are delegated to user1; however, while both
> subtrees may belong to the same user, it is clear that the two
> subtrees are to be isolated - they're under completely separate
> resource limits imposed by 0 and 1, respectively. Note that 0 and 1
> aren't strictly necessary but added to ease illustrating the issue.
>
> If user1 is allowed to move processes between the two subtrees, the
> intention of the hierarchy - keeping a given group of processes under
> a subtree with certain resource restrictions while delegating
> management of the subtree - can be circumvented by user1.
>
> This happens because migration permission check doesn't consider the
> hierarchical nature of cgroups. To fix the issue, this patch adds an
> extra permission requirement when userland tries to migrate a process
> in the default hierarchy - the issuing task must have write access to
> the common ancestor of "cgroup.procs" file of the ancestor in addition
> to the destination's.
>
> Conceptually, the issuer must be able to move the target process from
> the source cgroup to the common ancestor of source and destination
> cgroups and then to the destination. As long as delegation is done in
> a proper top-down way, this guarantees that a delegatee can't smuggle
> processes across disjoint delegation domains.
>
> The next patch will add documentation on the delegation model on the
> default hierarchy.
>
> v2: Fixed missing !ret test. Spotted by Li Zefan.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Li Zefan <lizefan@huawei.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
To: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
kernel-team-b10kYP2dOMg@public.gmane.org
Subject: Re: [PATCH v2 3/4] cgroup: require write perm on common ancestor when moving processes on the default hierarchy
Date: Thu, 18 Jun 2015 15:04:25 -0400 [thread overview]
Message-ID: <20150618190425.GB2182@cmpxchg.org> (raw)
In-Reply-To: <20150618175927.GD12934-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
On Thu, Jun 18, 2015 at 01:59:27PM -0400, Tejun Heo wrote:
> On traditional hierarchies, if a task has write access to "tasks" or
> "cgroup.procs" file of a cgroup and its euid agrees with the target,
> it can move the target to the cgroup; however, consider the following
> scenario. The owner of each cgroup is in the parentheses.
>
> R (root) - 0 (root) - 00 (user1) - 000 (user1)
> | \ 001 (user1)
> \ 1 (root) - 10 (user1)
>
> The subtrees of 00 and 10 are delegated to user1; however, while both
> subtrees may belong to the same user, it is clear that the two
> subtrees are to be isolated - they're under completely separate
> resource limits imposed by 0 and 1, respectively. Note that 0 and 1
> aren't strictly necessary but added to ease illustrating the issue.
>
> If user1 is allowed to move processes between the two subtrees, the
> intention of the hierarchy - keeping a given group of processes under
> a subtree with certain resource restrictions while delegating
> management of the subtree - can be circumvented by user1.
>
> This happens because migration permission check doesn't consider the
> hierarchical nature of cgroups. To fix the issue, this patch adds an
> extra permission requirement when userland tries to migrate a process
> in the default hierarchy - the issuing task must have write access to
> the common ancestor of "cgroup.procs" file of the ancestor in addition
> to the destination's.
>
> Conceptually, the issuer must be able to move the target process from
> the source cgroup to the common ancestor of source and destination
> cgroups and then to the destination. As long as delegation is done in
> a proper top-down way, this guarantees that a delegatee can't smuggle
> processes across disjoint delegation domains.
>
> The next patch will add documentation on the delegation model on the
> default hierarchy.
>
> v2: Fixed missing !ret test. Spotted by Li Zefan.
>
> Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Acked-by: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
next prev parent reply other threads:[~2015-06-18 19:05 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-16 19:10 [PATCHSET cgroup/for-4.2] cgroup: require write perm on common ancestor for migration Tejun Heo
2015-06-16 19:10 ` Tejun Heo
2015-06-16 19:10 ` [PATCH 1/4] kernfs: make kernfs_get_inode() public Tejun Heo
2015-06-16 19:10 ` Tejun Heo
2015-06-16 20:58 ` Greg Kroah-Hartman
2015-06-16 20:58 ` Greg Kroah-Hartman
2015-06-16 19:10 ` [PATCH 2/4] cgroup: separate out cgroup_procs_write_permission() from __cgroup_procs_write() Tejun Heo
2015-06-16 19:10 ` Tejun Heo
2015-06-16 19:10 ` [PATCH 3/4] cgroup: require write perm on common ancestor when moving processes on the default hierarchy Tejun Heo
2015-06-16 19:10 ` Tejun Heo
2015-06-18 3:14 ` Zefan Li
2015-06-18 3:14 ` Zefan Li
2015-06-18 17:59 ` Tejun Heo
2015-06-18 17:59 ` [PATCH v2 " Tejun Heo
2015-06-18 19:04 ` Johannes Weiner [this message]
2015-06-18 19:04 ` Johannes Weiner
2015-06-16 19:10 ` [PATCH 4/4] cgroup: add delegation section to unified hierarchy documentation Tejun Heo
2015-06-16 19:10 ` Tejun Heo
2015-06-18 19:01 ` Johannes Weiner
2015-06-18 19:01 ` Johannes Weiner
2015-06-18 20:23 ` [PATCH v2 " Tejun Heo
2015-06-18 20:23 ` Tejun Heo
2015-06-18 20:46 ` Johannes Weiner
2015-06-18 20:46 ` Johannes Weiner
2015-06-18 20:55 ` [PATCHSET cgroup/for-4.2] cgroup: require write perm on common ancestor for migration Tejun Heo
2015-06-18 20:55 ` Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150618190425.GB2182@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=cgroups@vger.kernel.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan@huawei.com \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.