Messages in this thread Patch in this message | | | Date | Wed, 30 Aug 2017 17:55:45 -0700 | From | Tejun Heo <> | Subject | Re: [PATCH] cgroup: Fix potential race between cgroup_exit and migrate path |
| |
Hello, Neeraj.
On Wed, Aug 30, 2017 at 06:24:09PM +0530, Neeraj Upadhyay wrote: > There is a potential race between cgroup_exit() and the > migration path. This race happens because cgroup_exit path > reads the css_set and does cg_list empty check outside of > css_set lock. This can potentially race with the migrate path > trying to move the tasks to a different css_set. For instance, > below is the interleaved sequence of events, where race is > observed: > > cpuset_hotplug_workfn() > cgroup_transfer_tasks() > cgroup_migrate() > cgroup_migrate_execute() > css_set_move_task() > list_del_init(&task->cg_list); > <TASK EXIT> > cgroup_exit() > cset = task_css_set(tsk); > if (!list_empty(&tsk->cg_list)) > <TASK NOT DISSOCIATED FROM ITS CSS_SET> > list_add_tail(&task->cg_list, use_mg_tasks > > In above sequence, as cgroup_exit() read the cg_list for > the task as empty, it didn't disassociate it from its > current css_set, and was moved to new css_set instance > css_set_move_task() called from cpuset_hotplug_workfn() > path. This eventually can result in use after free scenarios, > while accessing the same task_struct again, like in following > sequence: > > kernfs_seq_start() > cgroup_seqfile_start() > cgroup_pidlist_start() > css_task_iter_next() > __put_task_struct() > <NULL pointer dereference> > > Fix this problem, by moving the css_set and cg_list fetch in > cgroup_exit() inside css_set lock.
Hmm... I haven't really thought through but could the problem be that css_set_move_task() is temporarily making ->cg_list empty? The use_task_css_set_links optimization can't handle that.
Would something like the following fix the issue? Thanks.
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index df2e0f1..cd85ca0 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -683,7 +683,7 @@ static void css_set_move_task(struct task_struct *task, if (it->task_pos == &task->cg_list) css_task_iter_advance(it); - list_del_init(&task->cg_list); + list_del(&task->cg_list); if (!css_set_populated(from_cset)) css_set_update_populated(from_cset, false); } else {
| |