Skip to content

Commit

Permalink
cgroup: fix cgroup hierarchy umount race
Browse files Browse the repository at this point in the history
48ddbe1 "cgroup: make css->refcnt clearing on cgroup removal
optional" allowed a css to linger after the associated cgroup is
removed.  As a css holds a reference on the cgroup's dentry, it means
that cgroup dentries may linger for a while.

Destroying a superblock which has dentries with positive refcnts is a
critical bug and triggers BUG() in vfs code.  As each cgroup dentry
holds an s_active reference, any lingering cgroup has both its dentry
and the superblock pinned and thus preventing premature release of
superblock.

Unfortunately, after 48ddbe1, there's a small window while
releasing a cgroup which is directly under the root of the hierarchy.
When a cgroup directory is released, vfs layer first deletes the
corresponding dentry and then invokes dput() on the parent, which may
recurse further, so when a cgroup directly below root cgroup is
released, the cgroup is first destroyed - which releases the s_active
it was holding - and then the dentry for the root cgroup is dput().

This creates a window where the root dentry's refcnt isn't zero but
superblock's s_active is.  If umount happens before or during this
window, vfs will see the root dentry with non-zero refcnt and trigger
BUG().

Before 48ddbe1, this problem didn't exist because the last dentry
reference was guaranteed to be put synchronously from rmdir(2)
invocation which holds s_active around the whole process.

Fix it by holding an extra superblock->s_active reference across
dput() from css release, which is the dput() path added by 48ddbe1
and the only one which doesn't hold an extra s_active ref across the
final cgroup dput().

Signed-off-by: Tejun Heo <[email protected]>
LKML-Reference: <[email protected]>
Reported-by: shyju pv <[email protected]>
Tested-by: shyju pv <[email protected]>
Cc: Sasha Levin <[email protected]>
Acked-by: Li Zefan <[email protected]>
  • Loading branch information
htejun committed Jul 7, 2012
1 parent 7db5b3c commit 5db9a4d
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion kernel/cgroup.c
Original file line number Diff line number Diff line change
Expand Up @@ -3883,8 +3883,12 @@ static void css_dput_fn(struct work_struct *work)
{
struct cgroup_subsys_state *css =
container_of(work, struct cgroup_subsys_state, dput_work);
struct dentry *dentry = css->cgroup->dentry;
struct super_block *sb = dentry->d_sb;

dput(css->cgroup->dentry);
atomic_inc(&sb->s_active);
dput(dentry);
deactivate_super(sb);
}

static void init_cgroup_css(struct cgroup_subsys_state *css,
Expand Down

0 comments on commit 5db9a4d

Please sign in to comment.