cgroup: avoid attaching a cgroup root to two different superblocks, take 2
PATCH 1/1 cgroup: avoid attaching a cgroup root to two different superblocks, take 2
Commit bfb0b80db5f9 is broken. Now we try to fix the race by delaying the initialization of cgroup root refcnt until a superblock has been allocated.
---
I agree we apply this for 4.12.
Again I didn't receive your reply. I believe the problem is in my side. I'll see if there's some network administrator who can look into this issue, but I don't have much hope...
--- kernel/cgroup/cgroup-internal.h | 2 +- kernel/cgroup/cgroup-v1.c | 16 +++++++++++++++- kernel/cgroup/cgroup.c | 8 ++++---- 3 files changed, 20 insertions(+), 6 deletions(-)
kernel/cgroup/cgroup-internal.h +1 -1
@@ -163,7 +163,7 @@ int cgroup_path_ns_locked(struct cgroup *cgrp, char *buf, size_t buflen,void cgroup_free_root (struct cgroup_root *root);void init_cgroup_root (struct cgroup_root *root,struct cgroup_sb_opts *opts); -int cgroup_setup_root (struct cgroup_root *root,u16 ss_mask); +int cgroup_setup_root (struct cgroup_root *root,u16 ss_mask,int ref_flags);int rebind_subsystems (struct cgroup_root *dst_root,u16 ss_mask); struct dentry *cgroup_do_mount(struct file_system_type *fs_type, int flags, struct cgroup_root *root, unsigned long magic,
kernel/cgroup/cgroup-v1.c +15 -1
@@ -1072,6 +1072,7 @@ struct dentry *cgroup1_mount(struct file_system_type *fs_type, int flags,struct cgroup_subsys *ss;struct dentry *dentry;int i, ret; +bool new_root = false;cgroup_lock_and_drain_offline (&cgrp_dfl_root.cgrp);
@@ -1181,10 +1182,11 @@ struct dentry *cgroup1_mount(struct file_system_type *fs_type, int flags, ret = -ENOMEM ; goto out_unlock; } + new_root = true;init_cgroup_root (root, &opts); - ret =cgroup_setup_root (root, opts.subsys_mask); + ret =cgroup_setup_root (root, opts.subsys_mask,PERCPU_REF_INIT_DEAD );if (ret)cgroup_free_root (root);
@@ -1201,6 +1203,18 @@ struct dentry *cgroup1_mount(struct file_system_type *fs_type, int flags, CGROUP_SUPER_MAGIC, ns);/* +* There's a race window after we release cgroup_mutex and before +* allocating a superblock. Make sure a concurrent process won't +* be able to re-use the root during this window by delaying the +* initialization of root refcnt. +*/ +if (new_root) { +mutex_lock (&cgroup_mutex); +percpu_ref_reinit (&root->cgrp.self.refcnt); +mutex_unlock (&cgroup_mutex); + } + +/* * If @pinned_sb, we're reusing an existing root and holding an * extra ref on its sb. Mount is complete. Put the extra ref. */
kernel/cgroup/cgroup.c +4 -4
@@ -1640,7 +1640,7 @@ void init_cgroup_root(struct cgroup_root *root, struct cgroup_sb_opts *opts)set_bit (CGRP_CPUSET_CLONE_CHILDREN , &root->cgrp.flags); } -int cgroup_setup_root (struct cgroup_root *root,u16 ss_mask) +int cgroup_setup_root (struct cgroup_root *root,u16 ss_mask,int ref_flags) {LIST_HEAD (tmp_links);struct cgroup *root_cgrp = &root->cgrp;
@@ -1656,8 +1656,8 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask) root_cgrp->id = ret; root_cgrp->ancestor_ids[0 ] = ret; - ret =percpu_ref_init (&root_cgrp->self.refcnt, css_release,0 , -GFP_KERNEL ); + ret =percpu_ref_init (&root_cgrp->self.refcnt, css_release, + ref_flags,GFP_KERNEL );if (ret) goto out;
@@ -4512,7 +4512,7 @@ int __init cgroup_init(void)hash_add (css_set_table, &init_css_set.hlist,css_set_hash (init_css_set.subsys)); -BUG_ON (cgroup_setup_root (&cgrp_dfl_root,0 )); +BUG_ON (cgroup_setup_root (&cgrp_dfl_root,0 ,0 ));mutex_unlock (&cgroup_mutex);
Signature · 1 line
1.8.3.1
Quoted 9 lines · Zefan Li
Commit
bfb0b80db5f9is broken. Now we try to fix the race by delaying the initialization of cgroup root refcnt until a superblock has been allocated.Cc [email protected] # 3.16+Reported-by Dmitry Vyukov <[email protected]>Reported-by Andrei Vagin <[email protected]>Tested-by Andrei Vagin <[email protected]>Signed-off-by Zefan Li <[email protected]>
Applied to cgroup/for-4.12 w/o the stable tag. Let's wait this one out. It's unlikely to trigger in real use cases and if we decide that this needs to be in -stable, we can ask Greg later.
Quoted 3 lines
Again I didn't receive your reply. I believe the problem is in my side. I'll see if there's some network administrator who can look into this issue, but I don't have much hope...
Bummer. Is there another address I can cc? :(
Thanks.
Signature · 1 line
tejun