| From 48573a893303986e3b0b2974d6fb11f3d1bb7064 Mon Sep 17 00:00:00 2001 |
| From: Tejun Heo <tj@kernel.org> |
| Date: Sat, 8 Feb 2014 10:26:34 -0500 |
| Subject: cgroup: fix locking in cgroup_cfts_commit() |
| |
| From: Tejun Heo <tj@kernel.org> |
| |
| commit 48573a893303986e3b0b2974d6fb11f3d1bb7064 upstream. |
| |
| 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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| kernel/cgroup.c | 11 ++--------- |
| 1 file changed, 2 insertions(+), 9 deletions(-) |
| |
| --- a/kernel/cgroup.c |
| +++ b/kernel/cgroup.c |
| @@ -2845,10 +2845,7 @@ static int cgroup_cfts_commit(struct cft |
| */ |
| 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; |
| |
| @@ -2857,23 +2854,19 @@ static int cgroup_cfts_commit(struct cft |
| |
| 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; |