Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1536557 > unrolled thread
| Started by | John Stultz <john.stultz@linaro.org> |
|---|---|
| First post | 2016-12-06 01:30 +0100 |
| Last post | 2016-12-09 14:30 +0100 |
| Articles | 11 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups John Stultz <john.stultz@linaro.org> - 2016-12-06 01:30 +0100
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups Andy Lutomirski <luto@amacapital.net> - 2016-12-06 01:40 +0100
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups "Serge E. Hallyn" <serge@hallyn.com> - 2016-12-06 03:10 +0100
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups Tejun Heo <tj@kernel.org> - 2016-12-06 18:00 +0100
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups Tejun Heo <tj@kernel.org> - 2016-12-06 18:00 +0100
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups Andy Lutomirski <luto@amacapital.net> - 2016-12-06 18:10 +0100
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups Tejun Heo <tj@kernel.org> - 2016-12-06 19:20 +0100
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups Andy Lutomirski <luto@amacapital.net> - 2016-12-06 19:20 +0100
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups Tejun Heo <tj@kernel.org> - 2016-12-06 19:30 +0100
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups John Stultz <john.stultz@linaro.org> - 2016-12-09 06:40 +0100
Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups Tejun Heo <tj@kernel.org> - 2016-12-09 14:30 +0100
| From | John Stultz <john.stultz@linaro.org> |
|---|---|
| Date | 2016-12-06 01:30 +0100 |
| Subject | Re: [RESEND][PATCH v4] cgroup: Use CAP_SYS_RESOURCE to allow a process to migrate other tasks between cgroups |
| Message-ID | <sLbNT-3bh-19@gated-at.bofh.it> |
On Tue, Nov 22, 2016 at 4:57 PM, John Stultz <john.stultz@linaro.org> wrote: > On Tue, Nov 8, 2016 at 4:12 PM, Andy Lutomirski <luto@amacapital.net> wrote: >> On Tue, Nov 8, 2016 at 4:03 PM, Alexei Starovoitov >> <alexei.starovoitov@gmail.com> wrote: >>> On Tue, Nov 08, 2016 at 03:51:40PM -0800, Andy Lutomirski wrote: >>>> >>>> I hate to say it, but I think I may see a problem. Current >>>> developments are afoot to make cgroups do more than resource control. >>>> For example, there's Landlock and there's Daniel's ingress/egress >>>> filter thing. Current cgroup controllers can mostly just DoS their >>>> controlled processes. These new controllers (or controller-like >>>> things) can exfiltrate data and change semantics. >>>> >>>> Does anyone have a security model in mind for these controllers and >>>> the cgroups that they're attached to? I'm reasonably confident that >>>> CAP_SYS_RESOURCE is not the answer... >>> >>> and specifically the answer is... ? >>> Also would be great if you start with specifying the question first >>> and the problem you're trying to solve. >>> >> >> I don't have a good answer right now. Here are some constraints, though: >> >> 1. An insufficiently privileged process should not be able to move a >> victim into a dangerous cgroup. >> >> 2. An insufficiently privileged process should not be able to move >> itself into a dangerous cgroup and then use execve to gain privilege >> such that the execve'd program can be compromised. >> >> 3. An insufficiently privileged process should not be able to make an >> existing cgroup dangerous in a way that could compromise a victim in >> that cgroup. >> >> 4. An insufficiently privileged process should not be able to make a >> cgroup dangerous in a way that bypasses protections that would >> otherwise protect execve() as used by itself or some other process in >> that cgroup. >> >> Keep in mind that "dangerous" may apply to a cgroup's descendents in >> addition to the cgroup being controlled. > > Sorry for taking awhile to get back to you here. I'm a little > befuddled as to what next steps I should consider (and honestly, I'm > not totally sure I really grok your concern here, particularly what > you mean with "dangrous cgroups"). > > So is going back to the CAP_CGROUP_MIGRATE approach (to properly > separate "sufficiently" from "insufficiently privileged") better? > > Or something closer to the original method Android used of each cgroup > having an allow_attach() check which could determine what is > sufficiently privledged for the respective level of danger the cgroup > might poise? > > Or just stepping back, what method would you imagine to be reasonable > to allow a specified task to migrate other tasks between cgroups > without it having to be root/suid? Any suggested feedback here? thanks -john
[toc] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-12-06 01:40 +0100 |
| Message-ID | <sLbXA-3ei-9@gated-at.bofh.it> |
| In reply to | #1536557 |
On Mon, Dec 5, 2016 at 4:28 PM, John Stultz <john.stultz@linaro.org> wrote: > On Tue, Nov 22, 2016 at 4:57 PM, John Stultz <john.stultz@linaro.org> wrote: >> On Tue, Nov 8, 2016 at 4:12 PM, Andy Lutomirski <luto@amacapital.net> wrote: >>> On Tue, Nov 8, 2016 at 4:03 PM, Alexei Starovoitov >>> <alexei.starovoitov@gmail.com> wrote: >>>> On Tue, Nov 08, 2016 at 03:51:40PM -0800, Andy Lutomirski wrote: >>>>> >>>>> I hate to say it, but I think I may see a problem. Current >>>>> developments are afoot to make cgroups do more than resource control. >>>>> For example, there's Landlock and there's Daniel's ingress/egress >>>>> filter thing. Current cgroup controllers can mostly just DoS their >>>>> controlled processes. These new controllers (or controller-like >>>>> things) can exfiltrate data and change semantics. >>>>> >>>>> Does anyone have a security model in mind for these controllers and >>>>> the cgroups that they're attached to? I'm reasonably confident that >>>>> CAP_SYS_RESOURCE is not the answer... >>>> >>>> and specifically the answer is... ? >>>> Also would be great if you start with specifying the question first >>>> and the problem you're trying to solve. >>>> >>> >>> I don't have a good answer right now. Here are some constraints, though: >>> >>> 1. An insufficiently privileged process should not be able to move a >>> victim into a dangerous cgroup. >>> >>> 2. An insufficiently privileged process should not be able to move >>> itself into a dangerous cgroup and then use execve to gain privilege >>> such that the execve'd program can be compromised. >>> >>> 3. An insufficiently privileged process should not be able to make an >>> existing cgroup dangerous in a way that could compromise a victim in >>> that cgroup. >>> >>> 4. An insufficiently privileged process should not be able to make a >>> cgroup dangerous in a way that bypasses protections that would >>> otherwise protect execve() as used by itself or some other process in >>> that cgroup. >>> >>> Keep in mind that "dangerous" may apply to a cgroup's descendents in >>> addition to the cgroup being controlled. >> >> Sorry for taking awhile to get back to you here. I'm a little >> befuddled as to what next steps I should consider (and honestly, I'm >> not totally sure I really grok your concern here, particularly what >> you mean with "dangrous cgroups"). >> >> So is going back to the CAP_CGROUP_MIGRATE approach (to properly >> separate "sufficiently" from "insufficiently privileged") better? >> >> Or something closer to the original method Android used of each cgroup >> having an allow_attach() check which could determine what is >> sufficiently privledged for the respective level of danger the cgroup >> might poise? >> >> Or just stepping back, what method would you imagine to be reasonable >> to allow a specified task to migrate other tasks between cgroups >> without it having to be root/suid? > > Any suggested feedback here? I really don't know. The cgroupfs interface is a bit unfortunate in that it doesn't really express the constraints. To safely migrate a task, ISTM you ought to have some form of privilege over the task *and* some form of privilege over the cgroup. cgroupfs only handles the latter. CAP_CGROUP_MIGRATE ought to be okay. Or maybe cgroupfs needs to gain a concept of "dangerous" cgroups and further restrict them and CAP_SYS_RESOURCE should be fine for non-dangerous cgroups? I think I favor the latter, but it might be nice to hear from Tejun first. --Andy
[toc] | [prev] | [next] | [standalone]
| From | "Serge E. Hallyn" <serge@hallyn.com> |
|---|---|
| Date | 2016-12-06 03:10 +0100 |
| Message-ID | <sLdmF-4aS-3@gated-at.bofh.it> |
| In reply to | #1536561 |
On Mon, Dec 05, 2016 at 04:36:51PM -0800, Andy Lutomirski wrote: > On Mon, Dec 5, 2016 at 4:28 PM, John Stultz <john.stultz@linaro.org> wrote: > > On Tue, Nov 22, 2016 at 4:57 PM, John Stultz <john.stultz@linaro.org> wrote: > >> On Tue, Nov 8, 2016 at 4:12 PM, Andy Lutomirski <luto@amacapital.net> wrote: > >>> On Tue, Nov 8, 2016 at 4:03 PM, Alexei Starovoitov > >>> <alexei.starovoitov@gmail.com> wrote: > >>>> On Tue, Nov 08, 2016 at 03:51:40PM -0800, Andy Lutomirski wrote: > >>>>> > >>>>> I hate to say it, but I think I may see a problem. Current > >>>>> developments are afoot to make cgroups do more than resource control. > >>>>> For example, there's Landlock and there's Daniel's ingress/egress > >>>>> filter thing. Current cgroup controllers can mostly just DoS their > >>>>> controlled processes. These new controllers (or controller-like > >>>>> things) can exfiltrate data and change semantics. > >>>>> > >>>>> Does anyone have a security model in mind for these controllers and > >>>>> the cgroups that they're attached to? I'm reasonably confident that > >>>>> CAP_SYS_RESOURCE is not the answer... > >>>> > >>>> and specifically the answer is... ? > >>>> Also would be great if you start with specifying the question first > >>>> and the problem you're trying to solve. > >>>> > >>> > >>> I don't have a good answer right now. Here are some constraints, though: > >>> > >>> 1. An insufficiently privileged process should not be able to move a > >>> victim into a dangerous cgroup. > >>> > >>> 2. An insufficiently privileged process should not be able to move > >>> itself into a dangerous cgroup and then use execve to gain privilege > >>> such that the execve'd program can be compromised. > >>> > >>> 3. An insufficiently privileged process should not be able to make an > >>> existing cgroup dangerous in a way that could compromise a victim in > >>> that cgroup. > >>> > >>> 4. An insufficiently privileged process should not be able to make a > >>> cgroup dangerous in a way that bypasses protections that would > >>> otherwise protect execve() as used by itself or some other process in > >>> that cgroup. > >>> > >>> Keep in mind that "dangerous" may apply to a cgroup's descendents in > >>> addition to the cgroup being controlled. > >> > >> Sorry for taking awhile to get back to you here. I'm a little > >> befuddled as to what next steps I should consider (and honestly, I'm > >> not totally sure I really grok your concern here, particularly what > >> you mean with "dangrous cgroups"). > >> > >> So is going back to the CAP_CGROUP_MIGRATE approach (to properly > >> separate "sufficiently" from "insufficiently privileged") better? > >> > >> Or something closer to the original method Android used of each cgroup > >> having an allow_attach() check which could determine what is > >> sufficiently privledged for the respective level of danger the cgroup > >> might poise? > >> > >> Or just stepping back, what method would you imagine to be reasonable > >> to allow a specified task to migrate other tasks between cgroups > >> without it having to be root/suid? > > > > Any suggested feedback here? > > I really don't know. The cgroupfs interface is a bit unfortunate in > that it doesn't really express the constraints. To safely migrate a > task, ISTM you ought to have some form of privilege over the task > *and* some form of privilege over the cgroup. Agreed. The problem is that the privilege required should depend on the controller (I guess). For memory and cpuset, CAP_SYS_NICE seems right. Perhaps CAP_SYS_RESOURCE would be needed for some.. but then, as I look through the lists (capabilities(7) and the list of controllers), it seems like CAP_SYS_NICE works for everything. What else would we need? Maybe CAP_NET_ADMIN for net_cls and net_prio? CAP_SYS_RESOURCE|CAP_SYS_ADMIN for pids? > cgroupfs only handles > the latter. If we need different checks for different controllers, we can add checks to cgroupfs. > CAP_CGROUP_MIGRATE ought to be okay. Or maybe cgroupfs needs to gain > a concept of "dangerous" cgroups and further restrict them and > CAP_SYS_RESOURCE should be fine for non-dangerous cgroups? I think I > favor the latter, but it might be nice to hear from Tejun first. > > --Andy
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <tj@kernel.org> |
|---|---|
| Date | 2016-12-06 18:00 +0100 |
| Message-ID | <sLrfX-4AN-15@gated-at.bofh.it> |
| In reply to | #1536597 |
Hello, Serge. On Mon, Dec 05, 2016 at 08:00:11PM -0600, Serge E. Hallyn wrote: > > I really don't know. The cgroupfs interface is a bit unfortunate in > > that it doesn't really express the constraints. To safely migrate a > > task, ISTM you ought to have some form of privilege over the task > > *and* some form of privilege over the cgroup. > > Agreed. The problem is that the privilege required should depend on > the controller (I guess). For memory and cpuset, CAP_SYS_NICE seems > right. Perhaps CAP_SYS_RESOURCE would be needed for some.. but then, > as I look through the lists (capabilities(7) and the list of controllers), > it seems like CAP_SYS_NICE works for everything. What else would we need? > Maybe CAP_NET_ADMIN for net_cls and net_prio? CAP_SYS_RESOURCE|CAP_SYS_ADMIN > for pids? Please see my other reply but I don't think it's a good idea to have these extra checks on the side when there already is hierarchical delegation mechanism which should be able to handle both resource control and process management delegation. Thanks. -- tejun
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <tj@kernel.org> |
|---|---|
| Date | 2016-12-06 18:00 +0100 |
| Message-ID | <sLrfY-4AN-37@gated-at.bofh.it> |
| In reply to | #1536561 |
Hello, On Mon, Dec 05, 2016 at 04:36:51PM -0800, Andy Lutomirski wrote: > I really don't know. The cgroupfs interface is a bit unfortunate in > that it doesn't really express the constraints. To safely migrate a > task, ISTM you ought to have some form of privilege over the task > *and* some form of privilege over the cgroup. cgroupfs only handles > the latter. > > CAP_CGROUP_MIGRATE ought to be okay. Or maybe cgroupfs needs to gain > a concept of "dangerous" cgroups and further restrict them and > CAP_SYS_RESOURCE should be fine for non-dangerous cgroups? I think I > favor the latter, but it might be nice to hear from Tejun first. If we can't do CAP_SYS_RESOURCE due to overlaps, let's go with a separate CAP. While for android and cgroup v1, it's nice to have a finer grained CAP for security control, privilege isolation in cgroup should also primarily done through hierarchical delegation. It doesn't make sense to have another system in parallel. We can't do it properly on v1 because some controllers aren't properly hierarchical and delegation model isn't well defined. e.g. nothing prevents a process from being pulled across different subtrees with the same delegation, but v2 can do it properly. All that's necessary is to make the CAP test OR'd to other perm checks instead of AND'ing so that the CAP just allows overriding restrictions expressed through delegation but it's normally possible to move processes around in one's own delegated subtree. Thanks. -- tejun
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-12-06 18:10 +0100 |
| Message-ID | <sLrpE-4T9-5@gated-at.bofh.it> |
| In reply to | #1537116 |
On Tue, Dec 6, 2016 at 8:55 AM, Tejun Heo <tj@kernel.org> wrote: > Hello, > > On Mon, Dec 05, 2016 at 04:36:51PM -0800, Andy Lutomirski wrote: >> I really don't know. The cgroupfs interface is a bit unfortunate in >> that it doesn't really express the constraints. To safely migrate a >> task, ISTM you ought to have some form of privilege over the task >> *and* some form of privilege over the cgroup. cgroupfs only handles >> the latter. >> >> CAP_CGROUP_MIGRATE ought to be okay. Or maybe cgroupfs needs to gain >> a concept of "dangerous" cgroups and further restrict them and >> CAP_SYS_RESOURCE should be fine for non-dangerous cgroups? I think I >> favor the latter, but it might be nice to hear from Tejun first. > > If we can't do CAP_SYS_RESOURCE due to overlaps, let's go with a > separate CAP. While for android and cgroup v1, it's nice to have a > finer grained CAP for security control, privilege isolation in cgroup > should also primarily done through hierarchical delegation. It > doesn't make sense to have another system in parallel. > > We can't do it properly on v1 because some controllers aren't properly > hierarchical and delegation model isn't well defined. e.g. nothing > prevents a process from being pulled across different subtrees with > the same delegation, but v2 can do it properly. All that's necessary > is to make the CAP test OR'd to other perm checks instead of AND'ing > so that the CAP just allows overriding restrictions expressed through > delegation but it's normally possible to move processes around in > one's own delegated subtree. How would one be granted the right to move processes around in one's own subtree? Are you imagining that, if you're in /a/b and you want to move a process that's currently in /a/b/c to /a/b/d then you're allowed to because the target process is in your tree? If so, I doubt this has the security properties you want -- namely, if you can cooperate with anyone in /, even if they're unprivileged, you can break it.
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <tj@kernel.org> |
|---|---|
| Date | 2016-12-06 19:20 +0100 |
| Message-ID | <sLsvo-5zJ-37@gated-at.bofh.it> |
| In reply to | #1537122 |
Hello, On Tue, Dec 06, 2016 at 09:01:17AM -0800, Andy Lutomirski wrote: > How would one be granted the right to move processes around in one's > own subtree? Through expicit delegation - chowning of the directory and cgroup.procs file. > Are you imagining that, if you're in /a/b and you want to move a > process that's currently in /a/b/c to /a/b/d then you're allowed to > because the target process is in your tree? If so, I doubt this has > the security properties you want -- namely, if you can cooperate with > anyone in /, even if they're unprivileged, you can break it. Delegation is an explicit operation and reflected in the ownership of the subdirectories and cgroup interface files in them. The subhierarchy containment is achieved by requiring the user who's trying to migrate a process to have write perm on cgroup.procs on the common ancestor of the source and target in addition to the target. So, a user who has a subhierarchy delegated to it can move processes around inside but not out of or into it. Also, if there are multiple delegated disjoint subhierarchies, processes can't be moved across them by the delegatee either. Thanks. -- tejun
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-12-06 19:20 +0100 |
| Message-ID | <sLsvo-5zJ-35@gated-at.bofh.it> |
| In reply to | #1537190 |
On Tue, Dec 6, 2016 at 10:12 AM, Tejun Heo <tj@kernel.org> wrote: > Hello, > > On Tue, Dec 06, 2016 at 09:01:17AM -0800, Andy Lutomirski wrote: >> How would one be granted the right to move processes around in one's >> own subtree? > > Through expicit delegation - chowning of the directory and > cgroup.procs file. > >> Are you imagining that, if you're in /a/b and you want to move a >> process that's currently in /a/b/c to /a/b/d then you're allowed to >> because the target process is in your tree? If so, I doubt this has >> the security properties you want -- namely, if you can cooperate with >> anyone in /, even if they're unprivileged, you can break it. > > Delegation is an explicit operation and reflected in the ownership of > the subdirectories and cgroup interface files in them. The > subhierarchy containment is achieved by requiring the user who's > trying to migrate a process to have write perm on cgroup.procs on the > common ancestor of the source and target in addition to the target. OK, I see what you're doing. That's interesting.
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <tj@kernel.org> |
|---|---|
| Date | 2016-12-06 19:30 +0100 |
| Message-ID | <sLsF4-5D5-21@gated-at.bofh.it> |
| In reply to | #1537192 |
Hello,
On Tue, Dec 06, 2016 at 10:13:53AM -0800, Andy Lutomirski wrote:
> > Delegation is an explicit operation and reflected in the ownership of
> > the subdirectories and cgroup interface files in them. The
> > subhierarchy containment is achieved by requiring the user who's
> > trying to migrate a process to have write perm on cgroup.procs on the
> > common ancestor of the source and target in addition to the target.
>
> OK, I see what you're doing. That's interesting.
It's something born out of usages of cgroup v1. People used it that
way (chowning files and directories) and combined with the uid checksn
it yielded something which is useful sometimes, but it always had
issues with hierarchical behaviors, which files to chmod and the weird
combination of uid checks. cgroup v2 has a clear delegation model but
the uid checks are still left in as not changing was the default.
It's not necessary and I'm thinking about queueing something like the
following in the next cycle.
As for the android CAP discussion, I think it'd be nice to share an
existing CAP but if we can't find a good one to share, let's create a
new one.
Thanks.
diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
index 4cc07ce..34b4b44 100644
--- a/Documentation/cgroup-v2.txt
+++ b/Documentation/cgroup-v2.txt
@@ -328,14 +328,12 @@ a process with a non-root euid to migrate a target process into a
cgroup by writing its PID to the "cgroup.procs" file, the following
conditions must be met.
-- The writer's euid must match either uid or suid of the target process.
-
- The writer must have write access to the "cgroup.procs" file.
- The writer must have write access to the "cgroup.procs" file of the
common ancestor of the source and destination cgroups.
-The above three constraints ensure that while a delegatee may migrate
+The above two constraints ensure that while a delegatee may migrate
processes around freely in the delegated sub-hierarchy it can't pull
in from or push out to outside the sub-hierarchy.
@@ -350,10 +348,10 @@ all processes under C0 and C1 belong to U0.
Let's also say U0 wants to write the PID of a process which is
currently in C10 into "C00/cgroup.procs". U0 has write access to the
-file and uid match on the process; however, the common ancestor of the
-source cgroup C10 and the destination cgroup C00 is above the points
-of delegation and U0 would not have write access to its "cgroup.procs"
-files and thus the write will be denied with -EACCES.
+file; however, the common ancestor of the source cgroup C10 and the
+destination cgroup C00 is above the points of delegation and U0 would
+not have write access to its "cgroup.procs" files and thus the write
+will be denied with -EACCES.
2-6. Guidelines
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 85bc9be..49384ff 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2854,12 +2854,18 @@ static int cgroup_procs_write_permission(struct task_struct *task,
* even if we're attaching all tasks in the thread group, we only
* need to check permissions on one of them.
*/
- if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
- !uid_eq(cred->euid, tcred->uid) &&
- !uid_eq(cred->euid, tcred->suid))
- ret = -EACCES;
- if (!ret && cgroup_on_dfl(dst_cgrp)) {
+ /* root is allowed to do anything */
+ if (uid_eq(cred->euid, GLOBAL_ROOT_UID))
+ goto out;
+
+ /*
+ * On v2, follow the delegation model. Inside a delegated subtree,
+ * the delegatee can move around the processes however it sees fit.
+ *
+ * On v1, a process should match one of the target's uids.
+ */
+ if (cgroup_on_dfl(dst_cgrp)) {
struct super_block *sb = of->file->f_path.dentry->d_sb;
struct cgroup *cgrp;
struct inode *inode;
@@ -2877,8 +2883,11 @@ static int cgroup_procs_write_permission(struct task_struct *task,
ret = inode_permission(inode, MAY_WRITE);
iput(inode);
}
+ } else if (!uid_eq(cred->euid, tcred->uid) &&
+ !uid_eq(cred->euid, tcred->suid)) {
+ ret = -EACCES;
}
-
+out:
put_cred(tcred);
return ret;
}
[toc] | [prev] | [next] | [standalone]
| From | John Stultz <john.stultz@linaro.org> |
|---|---|
| Date | 2016-12-09 06:40 +0100 |
| Message-ID | <sMm4y-7Lb-25@gated-at.bofh.it> |
| In reply to | #1537201 |
On Tue, Dec 6, 2016 at 10:23 AM, Tejun Heo <tj@kernel.org> wrote: > Hello, > > On Tue, Dec 06, 2016 at 10:13:53AM -0800, Andy Lutomirski wrote: >> > Delegation is an explicit operation and reflected in the ownership of >> > the subdirectories and cgroup interface files in them. The >> > subhierarchy containment is achieved by requiring the user who's >> > trying to migrate a process to have write perm on cgroup.procs on the >> > common ancestor of the source and target in addition to the target. >> >> OK, I see what you're doing. That's interesting. > > It's something born out of usages of cgroup v1. People used it that > way (chowning files and directories) and combined with the uid checksn > it yielded something which is useful sometimes, but it always had > issues with hierarchical behaviors, which files to chmod and the weird > combination of uid checks. cgroup v2 has a clear delegation model but > the uid checks are still left in as not changing was the default. > > It's not necessary and I'm thinking about queueing something like the > following in the next cycle. > > As for the android CAP discussion, I think it'd be nice to share an > existing CAP but if we can't find a good one to share, let's create a > new one. So just to clarify the discussion for my purposes and make sure I understood, per-cgroup CAP rules was not desired, and instead we should either utilize an existing cap (are there still objections to CAP_SYS_RESOURCE? - this isn't clear to me) or create a new one (ie, bring back the older CAP_CGROUP_MIGRATE patch). Tejun: Do you have a more finished version of your patch that I should add my changes on top of? thanks -john
[toc] | [prev] | [next] | [standalone]
| From | Tejun Heo <tj@kernel.org> |
|---|---|
| Date | 2016-12-09 14:30 +0100 |
| Message-ID | <sMtpn-3Ka-1@gated-at.bofh.it> |
| In reply to | #1539108 |
Hello, John. On Thu, Dec 08, 2016 at 09:39:38PM -0800, John Stultz wrote: > So just to clarify the discussion for my purposes and make sure I > understood, per-cgroup CAP rules was not desired, and instead we > should either utilize an existing cap (are there still objections to > CAP_SYS_RESOURCE? - this isn't clear to me) or create a new one (ie, > bring back the older CAP_CGROUP_MIGRATE patch). Let's create a new one. It looks to be a bit too different to share with an existing one. > Tejun: Do you have a more finished version of your patch that I should > add my changes on top of? Oh, just submit the patch on top of the current for-next. I can queue mine on top of yours. They are mostly orthogonal. Thanks. -- tejun
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web