Commit 7aae4db5 authored May 26, 2017 by Neeraj Upadhyay Committed by ivanmeler Oct 08, 2021

cgroup: Fix potential race between cgroup_exit and migrate path



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_taskset_migrate()
          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.

Change-Id: I66060b8faffc06ffb19e2c166013551d4c9cb746
Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org>

parent fd37bcd7

kernel/cgroup.c

+11 −8

Original line number	Diff line number	Diff line
		@@ -717,10 +717,10 @@ static void css_set_move_task(struct task_struct *task,

		if (to_cset) {
		/*
		* We are synchronized through cgroup_threadgroup_rwsem
		* against PF_EXITING setting such that we can't race
		* against cgroup_exit() changing the css_set to
		* init_css_set and dropping the old one.
		* We are synchronized through css_set_lock against
		* PF_EXITING setting such that we can't race against
		* cgroup_exit() disassociating the task from the
		* css_set.
		*/
		WARN_ON_ONCE(task->flags & PF_EXITING);

		@@ -5704,19 +5704,22 @@ void cgroup_exit(struct task_struct *tsk)
		int i;

		/*
		* Unlink from @tsk from its css_set. As migration path can't race
		* with us, we can check css_set and cg_list without synchronization.
		* Avoid potential race with the migrate path.
		*/
		spin_lock_irq(&css_set_lock);
		/*
		* Unlink from @tsk from its css_set.
		*/
		cset = task_css_set(tsk);

		if (!list_empty(&tsk->cg_list)) {
		spin_lock_irq(&css_set_lock);
		css_set_move_task(tsk, cset, NULL, false);
		spin_unlock_irq(&css_set_lock);
		} else {
		get_css_set(cset);
		}

		spin_unlock_irq(&css_set_lock);

		/* see cgroup_post_fork() for details */
		for_each_subsys_which(ss, i, &have_exit_callback)
		ss->exit(tsk);