diff options
author | Li Zefan <lizefan@huawei.com> | 2014-09-04 02:43:07 -0400 |
---|---|---|
committer | Tejun Heo <tj@kernel.org> | 2014-09-04 12:36:18 -0400 |
commit | a4189487da1b4f8260c6006b9dc47c3c4107a5ae (patch) | |
tree | f58a1f62c4f9282659b5f84bf53888ed9ffab0ab | |
parent | fa8137be6ba632041e725e4623258ba27a2cf9be (diff) |
cgroup: delay the clearing of cgrp->kn->priv
Run these two scripts concurrently:
for ((; ;))
{
mkdir /cgroup/sub
rmdir /cgroup/sub
}
for ((; ;))
{
echo $$ > /cgroup/sub/cgroup.procs
echo $$ > /cgroup/cgroup.procs
}
A kernel bug will be triggered:
BUG: unable to handle kernel NULL pointer dereference at 00000038
IP: [<c10bbd69>] cgroup_put+0x9/0x80
...
Call Trace:
[<c10bbe19>] cgroup_kn_unlock+0x39/0x50
[<c10bbe91>] cgroup_kn_lock_live+0x61/0x70
[<c10be3c1>] __cgroup_procs_write.isra.26+0x51/0x230
[<c10be5b2>] cgroup_tasks_write+0x12/0x20
[<c10bb7b0>] cgroup_file_write+0x40/0x130
[<c11aee71>] kernfs_fop_write+0xd1/0x160
[<c1148e58>] vfs_write+0x98/0x1e0
[<c114934d>] SyS_write+0x4d/0xa0
[<c16f656b>] sysenter_do_call+0x12/0x12
We clear cgrp->kn->priv in the end of cgroup_rmdir(), but another
concurrent thread can access kn->priv after the clearing.
We should move the clearing to css_release_work_fn(). At that time
no one is holding reference to the cgroup and no one can gain a new
reference to access it.
v2:
- move RCU_INIT_POINTER() into the else block. (Tejun)
- remove the cgroup_parent() check. (Tejun)
- update the comment in css_tryget_online_from_dir().
Cc: <stable@vger.kernel.org> # 3.15+
Reported-by: Toralf Förster <toralf.foerster@gmx.de>
Signed-off-by: Zefan Li <lizefan@huawei.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
-rw-r--r-- | kernel/cgroup.c | 21 |
1 files changed, 10 insertions, 11 deletions
diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 50b94113f4f7..bf30076664ca 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c | |||
@@ -4396,6 +4396,15 @@ static void css_release_work_fn(struct work_struct *work) | |||
4396 | /* cgroup release path */ | 4396 | /* cgroup release path */ |
4397 | cgroup_idr_remove(&cgrp->root->cgroup_idr, cgrp->id); | 4397 | cgroup_idr_remove(&cgrp->root->cgroup_idr, cgrp->id); |
4398 | cgrp->id = -1; | 4398 | cgrp->id = -1; |
4399 | |||
4400 | /* | ||
4401 | * There are two control paths which try to determine | ||
4402 | * cgroup from dentry without going through kernfs - | ||
4403 | * cgroupstats_build() and css_tryget_online_from_dir(). | ||
4404 | * Those are supported by RCU protecting clearing of | ||
4405 | * cgrp->kn->priv backpointer. | ||
4406 | */ | ||
4407 | RCU_INIT_POINTER(*(void __rcu __force **)&cgrp->kn->priv, NULL); | ||
4399 | } | 4408 | } |
4400 | 4409 | ||
4401 | mutex_unlock(&cgroup_mutex); | 4410 | mutex_unlock(&cgroup_mutex); |
@@ -4834,16 +4843,6 @@ static int cgroup_rmdir(struct kernfs_node *kn) | |||
4834 | 4843 | ||
4835 | cgroup_kn_unlock(kn); | 4844 | cgroup_kn_unlock(kn); |
4836 | 4845 | ||
4837 | /* | ||
4838 | * There are two control paths which try to determine cgroup from | ||
4839 | * dentry without going through kernfs - cgroupstats_build() and | ||
4840 | * css_tryget_online_from_dir(). Those are supported by RCU | ||
4841 | * protecting clearing of cgrp->kn->priv backpointer, which should | ||
4842 | * happen after all files under it have been removed. | ||
4843 | */ | ||
4844 | if (!ret) | ||
4845 | RCU_INIT_POINTER(*(void __rcu __force **)&kn->priv, NULL); | ||
4846 | |||
4847 | cgroup_put(cgrp); | 4846 | cgroup_put(cgrp); |
4848 | return ret; | 4847 | return ret; |
4849 | } | 4848 | } |
@@ -5430,7 +5429,7 @@ struct cgroup_subsys_state *css_tryget_online_from_dir(struct dentry *dentry, | |||
5430 | /* | 5429 | /* |
5431 | * This path doesn't originate from kernfs and @kn could already | 5430 | * This path doesn't originate from kernfs and @kn could already |
5432 | * have been or be removed at any point. @kn->priv is RCU | 5431 | * have been or be removed at any point. @kn->priv is RCU |
5433 | * protected for this access. See cgroup_rmdir() for details. | 5432 | * protected for this access. See css_release_work_fn() for details. |
5434 | */ | 5433 | */ |
5435 | cgrp = rcu_dereference(kn->priv); | 5434 | cgrp = rcu_dereference(kn->priv); |
5436 | if (cgrp) | 5435 | if (cgrp) |