cgroup: fix exit() vsrmdir() race
PATCH 1/2 cgroup: fix exit() vsrmdir() race
In cgroup_exit() put_css_set_taskexit() is called without any lock, which might lead to accessing a freed cgroup:
thread1 thread2
---------------------------------------------exit()
cgroup_exit()
put_css_set_taskexit()
atomic_dec(cgrp->count);
rmdir();
/* not safe !! */
check_for_release(cgrp);rcu_read_lock() can be used to make sure the cgroup is alive.
--- kernel/cgroup.c | 8 ++++++++ 1 file changed, 8 insertions(+)
kernel/cgroup.c +8 -0
@@ -422,12 +422,20 @@ static void __put_css_set(struct css_set *cg, int taskexit)struct cgroup *cgrp = link->cgrp;list_del (&link->cg_link_list);list_del (&link->cgrp_link_list); + +/* +* We may not be holding cgroup_mutex, and if cgrp->count is +* dropped to 0 the cgroup can be destroyed at any time, hence +* rcu_read_lock is used to keep it alive. +*/ +rcu_read_lock ();if (atomic_dec_and_test (&cgrp->count) &¬ify_on_release (cgrp)) {if (taskexit)set_bit (CGRP_RELEASABLE , &cgrp->flags);check_for_release (cgrp); } +rcu_read_unlock ();kfree (link); }
Signature · 1 line
1.8.0.2
PATCH 2/2 cgroup: remove a NULL check in cgroup_exit()
init_task.cgroups is initialized at boot phase, and whenver a ask is forked, it's cgroups pointer is inherited from its parent, and it's never set to NULL afterwards.
--- kernel/cgroup.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
kernel/cgroup.c +1 -2
@@ -5011,8 +5011,7 @@ void cgroup_exit(struct task_struct *tsk, int run_callbacks) }task_unlock (tsk); -if (cg) -put_css_set_taskexit (cg); +put_css_set_taskexit (cg); }/**
Signature · 1 line
1.8.0.2
Re: [PATCH 2/2] cgroup: remove a NULL check in cgroup_exit()
Quoted 5 lines · Li Zefan
init_task.cgroups is initialized at boot phase, and whenver a ask is forked, it's cgroups pointer is inherited from its parent, and it's never set to NULL afterwards.
Signed-off-by Li Zefan <[email protected]>
Applied to cgroup/for-3.9.
Thanks.
Signature · 1 line
tejun
Tejun Heo
maintainerQuoted 13 lines · Li Zefan
In
cgroup_exit()put_css_set_taskexit()is called without any lock, which might lead to accessing a freed cgroup:thread1 thread2
---------------------------------------------exit()
cgroup_exit()put_css_set_taskexit() atomic_dec(cgrp->count); rmdir(); /* not safe !! */ check_for_release(cgrp);
Quoted 3 lines
rcu_read_lock()can be used to make sure the cgroup is alive.Signed-off-by Li Zefan <[email protected]>
Applying to cgroup/for-3.8-fixes w/ stable cc'd.
* Can you please add WARN_ON_ONCE(!rcu_read_lock_held()) in check_for_release()?
* cgroup_release_agent() seems broken too. It's accessing cgrp after removing it from release_list. Can you please fix it too?
Thanks.
Signature · 1 line
tejun
Tejun Heo
maintainerQuoted 2 lines · Tejun Heo
*
cgroup_release_agent()seems broken too. It's accessing cgrp after removing it fromrelease_list. Can you please fix it too?
Oh, we should be okay as we're holding cgroup_mutex there. Please forget about the above.
Thanks.
Signature · 1 line
tejun