From: "Serge E. Hallyn" <serge@hallyn.com> Date: 2016-03-21 23:41:38
One practical problem I've found with cgroup namespaces is that there
is no way to disambiguate between a cgroupfs mount which was done in
a cgroup namespace, and a bind mount of a cgroupfs directory. So
whether I do
unshare --cgroup -- bash -c "mount -t cgroup -o freezer f /mnt; cat /proc/self/mountinfo"
or whether I just
mount --bind /sys/fs/cgroup/freezer/$(awk -F: '/freezer/ { print $3 }' /proc/self/cgroup) /mnt
'mount root' field (field 3) in /proc/self/mountinfo will show the
same thing, the result of awk -F: '/freezer/ { print $3 }' /proc/self/cgroup.
This patch adds a 'nsroot=' field to cgroup mountinfo entries, so that
userspace can distinguish a mount made in a cgroup namespace from a bind
mount from a cgroup subdirectory.
Signed-off-by: Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
---
fs/kernfs/mount.c | 2 +-
include/linux/kernfs.h | 3 ++-
kernel/cgroup.c | 29 ++++++++++++++++++++++++++++-
3 files changed, 31 insertions(+), 3 deletions(-)
From: "Serge E. Hallyn" <serge@hallyn.com> Date: 2016-03-29 01:12:09
Quoting Serge E. Hallyn (serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org):
One practical problem I've found with cgroup namespaces is that there
is no way to disambiguate between a cgroupfs mount which was done in
a cgroup namespace, and a bind mount of a cgroupfs directory. So
whether I do
unshare --cgroup -- bash -c "mount -t cgroup -o freezer f /mnt; cat /proc/self/mountinfo"
or whether I just
mount --bind /sys/fs/cgroup/freezer/$(awk -F: '/freezer/ { print $3 }' /proc/self/cgroup) /mnt
'mount root' field (field 3) in /proc/self/mountinfo will show the
same thing, the result of awk -F: '/freezer/ { print $3 }' /proc/self/cgroup.
This patch adds a 'nsroot=' field to cgroup mountinfo entries, so that
userspace can distinguish a mount made in a cgroup namespace from a bind
mount from a cgroup subdirectory.
Hi Tejun,
no rush on the patch itself, I don't mind if i have to rewrite it from
scratch, but I'd like to get the patch into docker/libcontainer using it,
so can we decide on whether the syntax as shown here is ok?
thanks,
-serge
Hi Serge,
On Mon, Mar 21, 2016 at 06:41:33PM -0500, Serge E. Hallyn wrote:
One practical problem I've found with cgroup namespaces is that there
is no way to disambiguate between a cgroupfs mount which was done in
a cgroup namespace, and a bind mount of a cgroupfs directory. So
whether I do
unshare --cgroup -- bash -c "mount -t cgroup -o freezer f /mnt; cat /proc/self/mountinfo"
or whether I just
mount --bind /sys/fs/cgroup/freezer/$(awk -F: '/freezer/ { print $3 }' /proc/self/cgroup) /mnt
'mount root' field (field 3) in /proc/self/mountinfo will show the
same thing, the result of awk -F: '/freezer/ { print $3 }' /proc/self/cgroup.
This patch adds a 'nsroot=' field to cgroup mountinfo entries, so that
userspace can distinguish a mount made in a cgroup namespace from a bind
mount from a cgroup subdirectory.
With this patch, mountinfo shows nsroot= in the mount options, but the
actual mount() call for cgroups doesn't allow nsroot. Would it be
possible to allow passing nsroot= to mount, as long is it does in fact
match the current nsroot?
The motivation for this is that CRIU just copies the mount options and
uses them on restore, so with this patch we have to add a special case
to trim off nsroot= before we restore.
Tycho
Hi Serge,
On Mon, Mar 21, 2016 at 06:41:33PM -0500, Serge E. Hallyn wrote:
quoted
One practical problem I've found with cgroup namespaces is that there
is no way to disambiguate between a cgroupfs mount which was done in
a cgroup namespace, and a bind mount of a cgroupfs directory. So
whether I do
unshare --cgroup -- bash -c "mount -t cgroup -o freezer f /mnt; cat /proc/self/mountinfo"
or whether I just
mount --bind /sys/fs/cgroup/freezer/$(awk -F: '/freezer/ { print $3 }' /proc/self/cgroup) /mnt
'mount root' field (field 3) in /proc/self/mountinfo will show the
same thing, the result of awk -F: '/freezer/ { print $3 }' /proc/self/cgroup.
This patch adds a 'nsroot=' field to cgroup mountinfo entries, so that
userspace can distinguish a mount made in a cgroup namespace from a bind
mount from a cgroup subdirectory.
With this patch, mountinfo shows nsroot= in the mount options, but the
actual mount() call for cgroups doesn't allow nsroot. Would it be
possible to allow passing nsroot= to mount, as long is it does in fact
match the current nsroot?
Yeah, that should be possible. I'll try to send a patch for that later
this week. That's not to say Tejun will be ok with the behavior, but
it seems to make sense to me.
The motivation for this is that CRIU just copies the mount options and
uses them on restore, so with this patch we have to add a special case
to trim off nsroot= before we restore.
Tycho
From: "Serge E. Hallyn" <serge@hallyn.com> Date: 2016-03-30 17:21:06
As of the patch "cgroup namespaces: add a 'nsroot=' mountinfo field",
cgroupfs mountinfo output shows 'nsroot='. If userspace like criu
copy/pastes mount options from there into a new mount command, we should
ignore it.
Signed-off-by: Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
---
kernel/cgroup.c | 4 ++++
1 file changed, 4 insertions(+)
On Wed, Mar 30, 2016 at 12:21:00PM -0500, Serge E. Hallyn wrote:
As of the patch "cgroup namespaces: add a 'nsroot=' mountinfo field",
cgroupfs mountinfo output shows 'nsroot='. If userspace like criu
copy/pastes mount options from there into a new mount command, we should
ignore it.
Signed-off-by: Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
Hello, Serge.
On Mon, Mar 28, 2016 at 08:12:03PM -0500, Serge E. Hallyn wrote:
Quoting Serge E. Hallyn (serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org):
quoted
One practical problem I've found with cgroup namespaces is that there
is no way to disambiguate between a cgroupfs mount which was done in
a cgroup namespace, and a bind mount of a cgroupfs directory. So
whether I do
unshare --cgroup -- bash -c "mount -t cgroup -o freezer f /mnt; cat /proc/self/mountinfo"
or whether I just
mount --bind /sys/fs/cgroup/freezer/$(awk -F: '/freezer/ { print $3 }' /proc/self/cgroup) /mnt
'mount root' field (field 3) in /proc/self/mountinfo will show the
same thing, the result of awk -F: '/freezer/ { print $3 }' /proc/self/cgroup.
This patch adds a 'nsroot=' field to cgroup mountinfo entries, so that
userspace can distinguish a mount made in a cgroup namespace from a bind
mount from a cgroup subdirectory.
no rush on the patch itself, I don't mind if i have to rewrite it from
scratch, but I'd like to get the patch into docker/libcontainer using it,
so can we decide on whether the syntax as shown here is ok?
Yeah, I think the syntax is fine.
Thanks.
--
tejun
Hello, Serge.
Sorry about the delay.
On Mon, Mar 21, 2016 at 06:41:33PM -0500, Serge E. Hallyn wrote:
struct kernfs_syscall_ops {
int (*remount_fs)(struct kernfs_root *root, int *flags, char *data);
- int (*show_options)(struct seq_file *sf, struct kernfs_root *root);
+ int (*show_options)(struct seq_file *sf, struct dentry *dentry,
+ struct kernfs_root *root);
Wouldn't it make more sense to pass in kernfs_node pointer instead of
dentry pointer?
+static void cgroup_show_nsroot(struct seq_file *seq, struct dentry *dentry,
+ struct kernfs_root *kf_root)
+{
+ struct kernfs_node *d_kn = dentry->d_fsdata;
+ char *nsroot;
+ int len, ret;
+
+ if (!kf_root)
+ return;
+ len = kernfs_path_from_node(d_kn, kf_root->kn, NULL, 0);
+ if (len <= 0)
+ return;
+ nsroot = kzalloc(len + 1, GFP_ATOMIC);
+ if (!nsroot)
+ return;
+ ret = kernfs_path_from_node(d_kn, kf_root->kn, nsroot, len + 1);
+ if (ret <= 0 || ret > len)
+ goto out;
Hmmm.... does this mean that someone inside cgroup ns would be able to
find out the absolute cgroup path of the ns root from inside? If so,
wouldn't that be an unnecessary information leak?
Thanks.
--
tejun
Hello, Serge.
Sorry about the delay.
On Mon, Mar 21, 2016 at 06:41:33PM -0500, Serge E. Hallyn wrote:
quoted
struct kernfs_syscall_ops {
int (*remount_fs)(struct kernfs_root *root, int *flags, char *data);
- int (*show_options)(struct seq_file *sf, struct kernfs_root *root);
+ int (*show_options)(struct seq_file *sf, struct dentry *dentry,
+ struct kernfs_root *root);
Wouldn't it make more sense to pass in kernfs_node pointer instead of
dentry pointer?
Yeah that definately seems better.
quoted
+static void cgroup_show_nsroot(struct seq_file *seq, struct dentry *dentry,
+ struct kernfs_root *kf_root)
+{
+ struct kernfs_node *d_kn = dentry->d_fsdata;
+ char *nsroot;
+ int len, ret;
+
+ if (!kf_root)
+ return;
+ len = kernfs_path_from_node(d_kn, kf_root->kn, NULL, 0);
+ if (len <= 0)
+ return;
+ nsroot = kzalloc(len + 1, GFP_ATOMIC);
+ if (!nsroot)
+ return;
+ ret = kernfs_path_from_node(d_kn, kf_root->kn, nsroot, len + 1);
+ if (ret <= 0 || ret > len)
+ goto out;
Hmmm.... does this mean that someone inside cgroup ns would be able to
find out the absolute cgroup path of the ns root from inside? If so,
wouldn't that be an unnecessary information leak?
It's not a leak of any information we're trying to hide. I realize
something like 8 years have passed, but I still basically go by the
ksummit guidance that containers are ok but the kernel's first priority
is to facilitate containers but not trick containers into thinking
they're not containerized. So long as the container is properly set
up, I don't think there's anything the workload could do with the
nsroot= info other than *know* that it is in a ns cgroup.
If we did change that guidance, there's a slew of proc info that we
could better virtualize :)
thanks,
-serge
Hello, Serge.
On Wed, Apr 13, 2016 at 01:46:39PM -0500, Serge E. Hallyn wrote:
It's not a leak of any information we're trying to hide. I realize
something like 8 years have passed, but I still basically go by the
ksummit guidance that containers are ok but the kernel's first priority
is to facilitate containers but not trick containers into thinking
they're not containerized. So long as the container is properly set
up, I don't think there's anything the workload could do with the
nsroot= info other than *know* that it is in a ns cgroup.
If we did change that guidance, there's a slew of proc info that we
could better virtualize :)
I see. I'm just wondering because the information here seems a bit
gratuituous. Isn't the only thing necessary telling whether the root
is bind mounted or namescoped? Wouldn't simple "nsroot" work for that
purpose?
Thanks.
--
tejun
Hello, Serge.
On Wed, Apr 13, 2016 at 01:46:39PM -0500, Serge E. Hallyn wrote:
quoted
It's not a leak of any information we're trying to hide. I realize
something like 8 years have passed, but I still basically go by the
ksummit guidance that containers are ok but the kernel's first priority
is to facilitate containers but not trick containers into thinking
they're not containerized. So long as the container is properly set
up, I don't think there's anything the workload could do with the
nsroot= info other than *know* that it is in a ns cgroup.
If we did change that guidance, there's a slew of proc info that we
could better virtualize :)
I see. I'm just wondering because the information here seems a bit
gratuituous. Isn't the only thing necessary telling whether the root
is bind mounted or namescoped? Wouldn't simple "nsroot" work for that
purpose?
I don't think so - we could be in a cgroup namespace but still have
access only to bind-mounted cgroups. So we need to compare the
superblock dentry root field to the nsroot= value.
Hello,
On Wed, Apr 13, 2016 at 02:01:52PM -0500, Serge E. Hallyn wrote:
I don't think so - we could be in a cgroup namespace but still have
access only to bind-mounted cgroups. So we need to compare the
superblock dentry root field to the nsroot= value.
Hello, Serge.
On Wed, Apr 13, 2016 at 01:46:39PM -0500, Serge E. Hallyn wrote:
quoted
It's not a leak of any information we're trying to hide. I realize
something like 8 years have passed, but I still basically go by the
ksummit guidance that containers are ok but the kernel's first priority
is to facilitate containers but not trick containers into thinking
they're not containerized. So long as the container is properly set
up, I don't think there's anything the workload could do with the
nsroot= info other than *know* that it is in a ns cgroup.
If we did change that guidance, there's a slew of proc info that we
could better virtualize :)
I see. I'm just wondering because the information here seems a bit
gratuituous. Isn't the only thing necessary telling whether the root
is bind mounted or namescoped? Wouldn't simple "nsroot" work for that
purpose?
I don't think so - we could be in a cgroup namespace but still have
access only to bind-mounted cgroups. So we need to compare the
superblock dentry root field to the nsroot= value.
Umm, I don't think this is such a good idea. The main purpose of
cgroup namespace was to prevent this exposure of system cgroup
hierarchy that used to happen because of /proc/self/cgroup. Wouldn't
showing that information in /proc/self/mountinfo defeat the purpose?
One practical problem I've found with cgroup namespaces is that there
is no way to disambiguate between a cgroupfs mount which was done in
a cgroup namespace, and a bind mount of a cgroupfs directory.
Thats actually by design, no? Namespaced apps should not know/care if
they are running inside namespace. If they can find it out today, its
just because of certain side-effects. I fear adding explicit "nsroot"
or something in /proc/self/mountinfo now becomes an API making it hard
to virtualize user-apps again.
--
Aditya
Hello, Serge.
On Wed, Apr 13, 2016 at 01:46:39PM -0500, Serge E. Hallyn wrote:
quoted
It's not a leak of any information we're trying to hide. I realize
something like 8 years have passed, but I still basically go by the
ksummit guidance that containers are ok but the kernel's first priority
is to facilitate containers but not trick containers into thinking
they're not containerized. So long as the container is properly set
up, I don't think there's anything the workload could do with the
nsroot= info other than *know* that it is in a ns cgroup.
If we did change that guidance, there's a slew of proc info that we
could better virtualize :)
I see. I'm just wondering because the information here seems a bit
gratuituous. Isn't the only thing necessary telling whether the root
is bind mounted or namescoped? Wouldn't simple "nsroot" work for that
purpose?
I don't think so - we could be in a cgroup namespace but still have
access only to bind-mounted cgroups. So we need to compare the
superblock dentry root field to the nsroot= value.
Umm, I don't think this is such a good idea. The main purpose of
cgroup namespace was to prevent this exposure of system cgroup
hierarchy that used to happen because of /proc/self/cgroup. Wouldn't
showing that information in /proc/self/mountinfo defeat the purpose?
I disagree. The primary purpose was to simplify init's job and to keep
cgroup mounts in sync with /proc/self/cgroup. So that userspace doesn't
have to look at /proc/self/cgroup and then try and figure out how that
relates to its actual cgroup mountpoints. It was not to *hide* the
information.
Field 3 already gives us the path, nsroot just tells us what part of
it we are namespaced under.
quoted
One practical problem I've found with cgroup namespaces is that there
is no way to disambiguate between a cgroupfs mount which was done in
a cgroup namespace, and a bind mount of a cgroupfs directory.
Thats actually by design, no? Namespaced apps should not know/care if
they are running inside namespace. If they can find it out today, its
No. If a workload isn't allowed to mount its own cgroups, and can only
see that freezer /lxc/x1 was mounted at /dev/cgroup (poorly done, but we
don't get to pass judgement or choose mountpoints for userspace), and
it sees /lxc/x1 in its freezer entry for /proc/self/cgroup, then it
cannot tell whether it should be using /dev/cgroup/tasks or
/dev/cgroup/lxc/tasks or /dev/cgroup/lxc/x1/tasks. That's a problem.
just because of certain side-effects. I fear adding explicit "nsroot"
or something in /proc/self/mountinfo now becomes an API making it hard
to virtualize user-apps again.
It doesn't make it hard to virtualize. The only complication would be
if you wanted to checkpoint/restart and reproduce the exact
/proc/self/mountinfo output. That's a bogus goal anyway, since the
restart could be in a different cgroup and field 3 would be different.
In contrast, not providing this makes it impossible for software to
deal with both cgroup namespace and any bind-mounted cgroups. Which
means any new docker (say) which can run in cgroup namespaces will
not be able to run under old (that is, anything currently released
except lxc 2.0) container managers. We're breaking all container
managers.
Now the other thing we could do would be to tweak field 3 in the
mountinfo output. That had been my first inclination, but the way
the mountinfo code is currently done makes that ... challenging.
-serge
From: "Serge E. Hallyn" <serge@hallyn.com> Date: 2016-04-14 04:04:41
This is so that userspace can distinguish a mount made in a cgroup
namespace from a bind mount from a cgroup subdirectory.
Signed-off-by: Serge Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
---
Changelog: 2016-04-13: pass kernfs_node rather than dentry to show_options
---
fs/kernfs/mount.c | 2 +-
include/linux/kernfs.h | 3 ++-
kernel/cgroup.c | 28 +++++++++++++++++++++++++++-
3 files changed, 30 insertions(+), 3 deletions(-)