Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1445613 > unrolled thread

[PATCH v1 0/3] cgroup: allow for unprivileged management

Started byAleksa Sarai <asarai@suse.de>
First post2016-07-18 18:20 +0200
Last post2016-07-25 20:50 +0200
Articles 9 on this page of 29 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v1 0/3] cgroup: allow for unprivileged management Aleksa Sarai <asarai@suse.de> - 2016-07-18 18:20 +0200
    [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants Aleksa Sarai <asarai@suse.de> - 2016-07-18 18:20 +0200
      Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-20 18:00 +0200
        Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Aleksa Sarai <asarai@suse.de> - 2016-07-21 01:00 +0200
          Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-21 01:10 +0200
            Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Aleksa Sarai <asarai@suse.de> - 2016-07-21 01:20 +0200
              Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-21 01:20 +0200
                Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Aleksa Sarai <asarai@suse.de> - 2016-07-21 09:50 +0200
                  Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Aleksa Sarai <asarai@suse.de> - 2016-07-21 16:40 +0200
                    Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-21 17:10 +0200
                    Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants "Serge E. Hallyn" <serge@hallyn.com> - 2016-07-21 17:10 +0200
                  Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants "Serge E. Hallyn" <serge@hallyn.com> - 2016-07-21 16:40 +0200
                    Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-07-21 17:00 +0200
                      Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-21 17:00 +0200
                        Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Aleksa Sarai <asarai@suse.de> - 2016-07-21 17:10 +0200
                          Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-21 17:10 +0200
                  Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-21 17:00 +0200
                    Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-07-21 17:10 +0200
                      Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-21 17:10 +0200
                        Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-07-21 17:20 +0200
                          Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-21 17:30 +0200
                            Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-07-21 17:40 +0200
                              Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-21 18:00 +0200
                                Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-07-21 20:20 +0200
                                  Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-21 23:10 +0200
                                Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Aleksa Sarai <asarai@suse.de> - 2016-07-22 10:30 +0200
                                  Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-25 20:40 +0200
                        Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Aleksa Sarai <asarai@suse.de> - 2016-07-22 10:20 +0200
                          Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for  direct descendants Tejun Heo <tj@kernel.org> - 2016-07-25 20:50 +0200

Page 2 of 2 — ← Prev page 1 [2]


#1447968 — Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants

FromTejun Heo <tj@kernel.org>
Date2016-07-21 17:30 +0200
SubjectRe: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants
Message-ID<rXo5c-Qr-23@gated-at.bofh.it>
In reply to#1447962
Hello, James.

On Thu, Jul 21, 2016 at 08:16:34AM -0700, James Bottomley wrote:
> > That'd be one side.  The other side is the one moving.  Let's say the
> > system admin thing wants to move all processe from A proper to B.  It
> > would do that by draining processes from A's procs file into B's and
> > even that is multistep and can race.
> 
> So the second part is that once we allow the creation of
> subdirectories, there's no unified tasks file, so there's no way of
> draining A proper without enumerating and descending into the cgroupns
> created subtrees in A?

Not that.  If it races, it will end up moving processes which are no
longer in A proper.  Such operations or distinctions might not be
meaningful under many circumstances but it'd be a pretty big hole in
the API to create and I can't really declare these holes are gonna be
okay with confidence.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]


#1447974 — Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants

FromJames Bottomley <James.Bottomley@HansenPartnership.com>
Date2016-07-21 17:40 +0200
SubjectRe: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants
Message-ID<rXoeS-TM-7@gated-at.bofh.it>
In reply to#1447968
On Thu, 2016-07-21 at 11:26 -0400, Tejun Heo wrote:
> Hello, James.
> 
> On Thu, Jul 21, 2016 at 08:16:34AM -0700, James Bottomley wrote:
> > > That'd be one side.  The other side is the one moving.  Let's say 
> > > the system admin thing wants to move all processe from A proper 
> > > to B.   It would do that by draining processes from A's procs 
> > > file into B's and even that is multistep and can race.
> > 
> > So the second part is that once we allow the creation of
> > subdirectories, there's no unified tasks file, so there's no way of
> > draining A proper without enumerating and descending into the 
> > cgroupns created subtrees in A?
> 
> Not that.  If it races, it will end up moving processes which are no
> longer in A proper.

So if I as the cgroup ns owner am moving a task from A to A_subdir, the
admin scanning tasks in all of A may miss this task in motion because
all the tasks files can't be scanned atomically?

James

[toc] | [prev] | [next] | [standalone]


#1447975 — Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants

FromTejun Heo <tj@kernel.org>
Date2016-07-21 18:00 +0200
SubjectRe: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants
Message-ID<rXoyd-107-1@gated-at.bofh.it>
In reply to#1447974
Hello, James.

On Thu, Jul 21, 2016 at 08:34:36AM -0700, James Bottomley wrote:
> So if I as the cgroup ns owner am moving a task from A to A_subdir, the
> admin scanning tasks in all of A may miss this task in motion because
> all the tasks files can't be scanned atomically?

So, the admin just wants to move processes from A and only A to B.  It
doesn't wanna interfere with processes in the subdirs or on-going ns
operations, but if the race occurs, both A -> B migration and ns
subdir operation would succeed and the end result would be something
neither expects.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]


#1448045 — Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants

FromJames Bottomley <James.Bottomley@HansenPartnership.com>
Date2016-07-21 20:20 +0200
SubjectRe: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants
Message-ID<rXqJI-2G4-23@gated-at.bofh.it>
In reply to#1447975
On Thu, 2016-07-21 at 11:50 -0400, Tejun Heo wrote:
> Hello, James.
> 
> On Thu, Jul 21, 2016 at 08:34:36AM -0700, James Bottomley wrote:
> > So if I as the cgroup ns owner am moving a task from A to A_subdir, 
> > the admin scanning tasks in all of A may miss this task in motion
> > because all the tasks files can't be scanned atomically?
> 
> So, the admin just wants to move processes from A and only A to B. 
>  It doesn't wanna interfere with processes in the subdirs or on-going 
> ns operations, but if the race occurs, both A -> B migration and ns
> subdir operation would succeed and the end result would be something
> neither expects.

OK so a theoretical (not saying it's implementable, we'll have to
explore that) way of fixing all of this is to have separate views of
the tree.  If the admin always saw everything in A, even if the
cgroupns had created subdirectories in its own namespace.  That way
there'd be no race ever in the admin's view (because it's the view they
created and would expect to see).  All sub cgroup activity would only
be visible to tasks in the new cgroupns (we'd probably have to have
them make this visible by mounting a new cgroup tree).

James

[toc] | [prev] | [next] | [standalone]


#1448141 — Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants

FromTejun Heo <tj@kernel.org>
Date2016-07-21 23:10 +0200
SubjectRe: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants
Message-ID<rXtoe-4vY-15@gated-at.bofh.it>
In reply to#1448045
Hello,

On Thu, Jul 21, 2016 at 11:16:42AM -0700, James Bottomley wrote:
> OK so a theoretical (not saying it's implementable, we'll have to
> explore that) way of fixing all of this is to have separate views of
> the tree.  If the admin always saw everything in A, even if the
> cgroupns had created subdirectories in its own namespace.  That way
> there'd be no race ever in the admin's view (because it's the view they
> created and would expect to see).  All sub cgroup activity would only
> be visible to tasks in the new cgroupns (we'd probably have to have
> them make this visible by mounting a new cgroup tree).

Yeah, something like that.  The two domains of operation need to be
transparent to each other so that things taking place at system level
doesn't interfere with user level operations and vice-versa.  It's
likely that implementing something like that within filesystem based
interface won't work out too well.  There are too many expected
behaviors from being a filesystem which don't quite agree with such
abstraction.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]


#1448459 — Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants

FromAleksa Sarai <asarai@suse.de>
Date2016-07-22 10:30 +0200
SubjectRe: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants
Message-ID<rXE0h-3hR-15@gated-at.bofh.it>
In reply to#1447975
>> So if I as the cgroup ns owner am moving a task from A to A_subdir, the
>> admin scanning tasks in all of A may miss this task in motion because
>> all the tasks files can't be scanned atomically?
>
> So, the admin just wants to move processes from A and only A to B.  It
> doesn't wanna interfere with processes in the subdirs or on-going ns
> operations, but if the race occurs, both A -> B migration and ns
> subdir operation would succeed and the end result would be something
> neither expects.

Just to be clear, the "ns subdir operation" is a cgroup namespaced 
process moving A -> A_subdir which is racing against some administrative 
process moving everything from A -> B (but not wanting to move A -> 
A_subdir)?

So should there be policy within the kernel to not permit a process 
outside a cgroup namespace to move processes inside the namespace? Or 
would you be concerned about people escaping the administrator's 
attempts to reorganise the hierarchy?

What if we extended rename(2) so that it /does/ allow for reorganisation 
of the hierarchy? So an administrator could use rename to change the 
point at which a cgroupns root is rooted at, but not be able to move the 
actual processes within the cgroup namespace around? The administrator 
could also join the cgroupns (without needing to join the userns) and 
then just move things around that way?

Do any of those suggestions seem reasonable?

-- 
Aleksa Sarai
Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/

[toc] | [prev] | [next] | [standalone]


#1449732 — Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants

FromTejun Heo <tj@kernel.org>
Date2016-07-25 20:40 +0200
SubjectRe: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants
Message-ID<rYSXg-7Yy-33@gated-at.bofh.it>
In reply to#1448459
Hello, Aleksa.

On Fri, Jul 22, 2016 at 06:30:07PM +1000, Aleksa Sarai wrote:
> Just to be clear, the "ns subdir operation" is a cgroup namespaced process
> moving A -> A_subdir which is racing against some administrative process
> moving everything from A -> B (but not wanting to move A -> A_subdir)?

Yes.

> So should there be policy within the kernel to not permit a process outside
> a cgroup namespace to move processes inside the namespace? Or would you be
> concerned about people escaping the administrator's attempts to reorganise
> the hierarchy?

Pushed that far, I frankly can't assess what the implications and
side-effects would be.

> What if we extended rename(2) so that it /does/ allow for reorganisation of
> the hierarchy? So an administrator could use rename to change the point at
> which a cgroupns root is rooted at, but not be able to move the actual
> processes within the cgroup namespace around? The administrator could also
> join the cgroupns (without needing to join the userns) and then just move
> things around that way?
> 
> Do any of those suggestions seem reasonable?

Unfortunately not.  I get what you're trying to do and am sure we can
make some specific scenarios work with the right set of hacks and
holes, but this type of approach is very dangerous in the long term.

The downside we have now is that we need an explicit delegation from
userland and that stems from the architectural constraints of
cgroupfs.  It's not ideal but an acceptable situation.  Let's please
not riddle the whole thing with holes that we don't understand for an
inconvenience which can be worked around otherwise.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]


#1448453 — Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants

FromAleksa Sarai <asarai@suse.de>
Date2016-07-22 10:20 +0200
SubjectRe: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants
Message-ID<rXDQC-3e4-5@gated-at.bofh.it>
In reply to#1447954
>>  It's about the debris left behind if the admin (or someone with
>> delegated authority) moves the task to a wholly different cgroup.
>>
>> Now we have a cgroup directory in the old cgroup, which the current
>> task has been removed from, for which the current user has permissions
>> and could then move the task back to.  Is that the essence of the
>> problem?
>
> That'd be one side.  The other side is the one moving.  Let's say the
> system admin thing wants to move all processe from A proper to B.  It
> would do that by draining processes from A's procs file into B's and
> even that is multistep and can race.

Once freezer is ported, wouldn't that allow you to stop the processes so 
you can drain them? I understand your concern with draining, but surely 
the same races occur if you fork? How many times would you need to scan 
cgroup.procs to make sure that you didn't miss anything (and if there's 
enough processes then cgroup.procs reads aren't atomic either).

-- 
Aleksa Sarai
Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/

[toc] | [prev] | [next] | [standalone]


#1449743 — Re: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants

FromTejun Heo <tj@kernel.org>
Date2016-07-25 20:50 +0200
SubjectRe: [PATCH v1 3/3] cgroup: relax common ancestor restriction for direct descendants
Message-ID<rYT6V-81R-19@gated-at.bofh.it>
In reply to#1448453
On Fri, Jul 22, 2016 at 06:24:25PM +1000, Aleksa Sarai wrote:
> > >  It's about the debris left behind if the admin (or someone with
> > > delegated authority) moves the task to a wholly different cgroup.
> > > 
> > > Now we have a cgroup directory in the old cgroup, which the current
> > > task has been removed from, for which the current user has permissions
> > > and could then move the task back to.  Is that the essence of the
> > > problem?
> > 
> > That'd be one side.  The other side is the one moving.  Let's say the
> > system admin thing wants to move all processe from A proper to B.  It
> > would do that by draining processes from A's procs file into B's and
> > even that is multistep and can race.
> 
> Once freezer is ported, wouldn't that allow you to stop the processes so you
> can drain them? I understand your concern with draining, but surely the same
> races occur if you fork? How many times would you need to scan cgroup.procs
> to make sure that you didn't miss anything (and if there's enough processes
> then cgroup.procs reads aren't atomic either).

Sure, draining has to be iterative and in actual use cases freezing
would help but I think you're misunderstanding why I gave the example.
It wasn't meant to be "if you solve this problem case, we're all
good".  It was an illustration of the underlying problems where the
basic interface isn't fit for this sort of complex semantics.  Please
take a step back and look at the larger picture.  The current
interface is silly in many areas but it's still mostly integral and
I'd really like to keep at least that.

If there is a way to do this in a way which is not hacky, we can
definitely look into it.  However, given that the imminent problem is
relatively small, I'm not too sure that there would be a solution of
justifiable size.

Thanks.

-- 
tejun

[toc] | [prev] | [standalone]


Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel


csiph-web