Commit 7d73a89b authored by Li Zefan's avatar Li Zefan Committed by Greg Kroah-Hartman

cgroup: delay the clearing of cgrp->kn->priv

commit a4189487 upstream.

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().
Reported-by: default avatarToralf Förster <toralf.foerster@gmx.de>
Signed-off-by: default avatarZefan Li <lizefan@huawei.com>
Signed-off-by: default avatarTejun Heo <tj@kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 767f5ccd
...@@ -4242,6 +4242,15 @@ static void css_release_work_fn(struct work_struct *work) ...@@ -4242,6 +4242,15 @@ static void css_release_work_fn(struct work_struct *work)
/* cgroup release path */ /* cgroup release path */
cgroup_idr_remove(&cgrp->root->cgroup_idr, cgrp->id); cgroup_idr_remove(&cgrp->root->cgroup_idr, cgrp->id);
cgrp->id = -1; cgrp->id = -1;
/*
* There are two control paths which try to determine
* cgroup from dentry without going through kernfs -
* cgroupstats_build() and css_tryget_online_from_dir().
* Those are supported by RCU protecting clearing of
* cgrp->kn->priv backpointer.
*/
RCU_INIT_POINTER(*(void __rcu __force **)&cgrp->kn->priv, NULL);
} }
mutex_unlock(&cgroup_mutex); mutex_unlock(&cgroup_mutex);
...@@ -4667,16 +4676,6 @@ static int cgroup_rmdir(struct kernfs_node *kn) ...@@ -4667,16 +4676,6 @@ static int cgroup_rmdir(struct kernfs_node *kn)
cgroup_kn_unlock(kn); cgroup_kn_unlock(kn);
/*
* There are two control paths which try to determine cgroup from
* dentry without going through kernfs - cgroupstats_build() and
* css_tryget_online_from_dir(). Those are supported by RCU
* protecting clearing of cgrp->kn->priv backpointer, which should
* happen after all files under it have been removed.
*/
if (!ret)
RCU_INIT_POINTER(*(void __rcu __force **)&kn->priv, NULL);
cgroup_put(cgrp); cgroup_put(cgrp);
return ret; return ret;
} }
...@@ -5242,7 +5241,7 @@ struct cgroup_subsys_state *css_tryget_online_from_dir(struct dentry *dentry, ...@@ -5242,7 +5241,7 @@ struct cgroup_subsys_state *css_tryget_online_from_dir(struct dentry *dentry,
/* /*
* This path doesn't originate from kernfs and @kn could already * This path doesn't originate from kernfs and @kn could already
* have been or be removed at any point. @kn->priv is RCU * have been or be removed at any point. @kn->priv is RCU
* protected for this access. See cgroup_rmdir() for details. * protected for this access. See css_release_work_fn() for details.
*/ */
cgrp = rcu_dereference(kn->priv); cgrp = rcu_dereference(kn->priv);
if (cgrp) if (cgrp)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment