diff options
author | Tejun Heo <tj@kernel.org> | 2014-02-08 10:26:34 -0500 |
---|---|---|
committer | Tejun Heo <tj@kernel.org> | 2014-02-08 10:26:34 -0500 |
commit | 48573a893303986e3b0b2974d6fb11f3d1bb7064 (patch) | |
tree | a01edf5728485e7637a25b08020f210a3ae0d67e /kernel/cgroup.c | |
parent | b58c89986a77a23658682a100eb15d8edb571ebb (diff) | |
download | lwn-48573a893303986e3b0b2974d6fb11f3d1bb7064.tar.gz lwn-48573a893303986e3b0b2974d6fb11f3d1bb7064.zip |
cgroup: fix locking in cgroup_cfts_commit()
cgroup_cfts_commit() walks the cgroup hierarchy that the target
subsystem is attached to and tries to apply the file changes. Due to
the convolution with inode locking, it can't keep cgroup_mutex locked
while iterating. It currently holds only RCU read lock around the
actual iteration and then pins the found cgroup using dget().
Unfortunately, this is incorrect. Although the iteration does check
cgroup_is_dead() before invoking dget(), there's nothing which
prevents the dentry from going away inbetween. Note that this is
different from the usual css iterations where css_tryget() is used to
pin the css - css_tryget() tests whether the css can be pinned and
fails if not.
The problem can be solved by simply holding cgroup_mutex instead of
RCU read lock around the iteration, which actually reduces LOC.
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Li Zefan <lizefan@huawei.com>
Cc: stable@vger.kernel.org
Diffstat (limited to 'kernel/cgroup.c')
-rw-r--r-- | kernel/cgroup.c | 11 |
1 files changed, 2 insertions, 9 deletions
diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 0eb7b868e1ab..3edf7163b84f 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -2763,10 +2763,7 @@ static int cgroup_cfts_commit(struct cftype *cfts, bool is_add) */ update_before = cgroup_serial_nr_next; - mutex_unlock(&cgroup_mutex); - /* add/rm files for all cgroups created before */ - rcu_read_lock(); css_for_each_descendant_pre(css, cgroup_css(root, ss)) { struct cgroup *cgrp = css->cgroup; @@ -2775,23 +2772,19 @@ static int cgroup_cfts_commit(struct cftype *cfts, bool is_add) inode = cgrp->dentry->d_inode; dget(cgrp->dentry); - rcu_read_unlock(); - dput(prev); prev = cgrp->dentry; + mutex_unlock(&cgroup_mutex); mutex_lock(&inode->i_mutex); mutex_lock(&cgroup_mutex); if (cgrp->serial_nr < update_before && !cgroup_is_dead(cgrp)) ret = cgroup_addrm_files(cgrp, cfts, is_add); - mutex_unlock(&cgroup_mutex); mutex_unlock(&inode->i_mutex); - - rcu_read_lock(); if (ret) break; } - rcu_read_unlock(); + mutex_unlock(&cgroup_mutex); dput(prev); deactivate_super(sb); return ret; |