Commit 2d3cbf8b authored by Tomasz Buchert's avatar Tomasz Buchert Committed by Linus Torvalds
Browse files

cgroup_freezer: update_freezer_state() does incorrect state transitions 

There are 4 state transitions possible for a freezer.  Only FREEZING ->
FROZEN transaction is done lazily.  This patch allows update_freezer_state
only to perform this transaction and renames the function to
update_if_frozen.

Moreover is_task_frozen_enough function is removed and its every occurence
is replaced with frozen().  Therefore for a group to become FROZEN every
task must be frozen.

The previous version could trigger a following bug: When cgroup is in the
process of freezing (but none of its tasks are frozen yet),
update_freezer_state() (called from freezer_read or freezer_write) would
incorrectly report that a group is 'THAWED' (because nfrozen = 0),
allowing the transaction FREEZING -> THAWED without writing anything to
'freezer.state'.  This is incorrect according to the documentation.  This
could result in a 'THAWED' cgroup with frozen tasks inside.

A code to reproduce this bug is available here:
http://pentium.hopto.org/~thinred/repos/linux-misc/freezer_bug2.c



[akpm@linux-foundation.org: coding-style fixes]
Signed-off-by: default avatarTomasz Buchert <tomasz.buchert@inria.fr>
Cc: Matt Helsley <matthltc@us.ibm.com>
Cc: Paul Menage <menage@google.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 0bdba580

kernel/cgroup_freezer.c

+15 −23
Original line number Diff line number Diff line
@@ -153,13 +153,6 @@ static void freezer_destroy(struct cgroup_subsys *ss, 
	kfree(cgroup_freezer(cgroup));

}



/* Task is frozen or will freeze immediately when next it gets woken */

static bool is_task_frozen_enough(struct task_struct *task)

{

	return frozen(task) ||

		(task_is_stopped_or_traced(task) && freezing(task));

}



/*

 * The call to cgroup_lock() in the freezer.state write method prevents

 * a write to that file racing against an attach, and hence the
@@ -236,31 +229,30 @@ static void freezer_fork(struct cgroup_subsys *ss, struct task_struct *task) 
/*

 * caller must hold freezer->lock

 */

static void update_freezer_state(struct cgroup *cgroup,

static void update_if_frozen(struct cgroup *cgroup,

				 struct freezer *freezer)

{

	struct cgroup_iter it;

	struct task_struct *task;

	unsigned int nfrozen = 0, ntotal = 0;

	enum freezer_state old_state = freezer->state;



	cgroup_iter_start(cgroup, &it);

	while ((task = cgroup_iter_next(cgroup, &it))) {

		ntotal++;

		if (is_task_frozen_enough(task))

		if (frozen(task))

			nfrozen++;

	}



	/*

	 * Transition to FROZEN when no new tasks can be added ensures

	 * that we never exist in the FROZEN state while there are unfrozen

	 * tasks.

	 */

	if (old_state == CGROUP_THAWED) {

		BUG_ON(nfrozen > 0);

	} else if (old_state == CGROUP_FREEZING) {

		if (nfrozen == ntotal)

			freezer->state = CGROUP_FROZEN;

	else if (nfrozen > 0)

		freezer->state = CGROUP_FREEZING;

	else

		freezer->state = CGROUP_THAWED;

	} else { /* old_state == CGROUP_FROZEN */

		BUG_ON(nfrozen != ntotal);

	}



	cgroup_iter_end(cgroup, &it);

}
@@ -279,7 +271,7 @@ static int freezer_read(struct cgroup *cgroup, struct cftype *cft, 
	if (state == CGROUP_FREEZING) {

		/* We change from FREEZING to FROZEN lazily if the cgroup was

		 * only partially frozen when we exitted write. */

		update_freezer_state(cgroup, freezer);

		update_if_frozen(cgroup, freezer);

		state = freezer->state;

	}

	spin_unlock_irq(&freezer->lock);
@@ -301,7 +293,7 @@ static int try_to_freeze_cgroup(struct cgroup *cgroup, struct freezer *freezer) 
	while ((task = cgroup_iter_next(cgroup, &it))) {

		if (!freeze_task(task, true))

			continue;

		if (is_task_frozen_enough(task))

		if (frozen(task))

			continue;

		if (!freezing(task) && !freezer_should_skip(task))

			num_cant_freeze_now++;
@@ -335,7 +327,7 @@ static int freezer_change_state(struct cgroup *cgroup, 


	spin_lock_irq(&freezer->lock);



	update_freezer_state(cgroup, freezer);

	update_if_frozen(cgroup, freezer);

	if (goal_state == freezer->state)

		goto out;

CRA Git | Maintained and supported by SUSTech CRA and CCSE