Commit 7805d000 authored by Tejun Heo's avatar Tejun Heo

cgroup: fix a subtle bug in descendant pre-order walk

When cgroup_next_descendant_pre() initiates a walk, it checks whether
the subtree root doesn't have any children and if not returns NULL.
Later code assumes that the subtree isn't empty.  This is broken
because the subtree may become empty inbetween, which can lead to the
traversal escaping the subtree by walking to the sibling of the
subtree root.

There's no reason to have the early exit path.  Remove it along with
the later assumption that the subtree isn't empty.  This simplifies
the code a bit and fixes the subtle bug.

While at it, fix the comment of cgroup_for_each_descendant_pre() which
was incorrectly referring to ->css_offline() instead of
->css_online().
Signed-off-by: default avatarTejun Heo <tj@kernel.org>
Reviewed-by: default avatarMichal Hocko <mhocko@suse.cz>
Cc: stable@vger.kernel.org
parent d6cbf35d
...@@ -707,7 +707,7 @@ struct cgroup *cgroup_rightmost_descendant(struct cgroup *pos); ...@@ -707,7 +707,7 @@ struct cgroup *cgroup_rightmost_descendant(struct cgroup *pos);
* *
* If a subsystem synchronizes against the parent in its ->css_online() and * If a subsystem synchronizes against the parent in its ->css_online() and
* before starting iterating, and synchronizes against @pos on each * before starting iterating, and synchronizes against @pos on each
* iteration, any descendant cgroup which finished ->css_offline() is * iteration, any descendant cgroup which finished ->css_online() is
* guaranteed to be visible in the future iterations. * guaranteed to be visible in the future iterations.
* *
* In other words, the following guarantees that a descendant can't escape * In other words, the following guarantees that a descendant can't escape
......
...@@ -2954,11 +2954,8 @@ struct cgroup *cgroup_next_descendant_pre(struct cgroup *pos, ...@@ -2954,11 +2954,8 @@ struct cgroup *cgroup_next_descendant_pre(struct cgroup *pos,
WARN_ON_ONCE(!rcu_read_lock_held()); WARN_ON_ONCE(!rcu_read_lock_held());
/* if first iteration, pretend we just visited @cgroup */ /* if first iteration, pretend we just visited @cgroup */
if (!pos) { if (!pos)
if (list_empty(&cgroup->children))
return NULL;
pos = cgroup; pos = cgroup;
}
/* visit the first child if exists */ /* visit the first child if exists */
next = list_first_or_null_rcu(&pos->children, struct cgroup, sibling); next = list_first_or_null_rcu(&pos->children, struct cgroup, sibling);
...@@ -2966,14 +2963,14 @@ struct cgroup *cgroup_next_descendant_pre(struct cgroup *pos, ...@@ -2966,14 +2963,14 @@ struct cgroup *cgroup_next_descendant_pre(struct cgroup *pos,
return next; return next;
/* no child, visit my or the closest ancestor's next sibling */ /* no child, visit my or the closest ancestor's next sibling */
do { while (pos != cgroup) {
next = list_entry_rcu(pos->sibling.next, struct cgroup, next = list_entry_rcu(pos->sibling.next, struct cgroup,
sibling); sibling);
if (&next->sibling != &pos->parent->children) if (&next->sibling != &pos->parent->children)
return next; return next;
pos = pos->parent; pos = pos->parent;
} while (pos != cgroup); }
return NULL; return NULL;
} }
......
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