diff options
author | Tejun Heo <tj@kernel.org> | 2014-03-20 11:10:15 -0400 |
---|---|---|
committer | Tejun Heo <tj@kernel.org> | 2014-03-20 11:10:15 -0400 |
commit | e1b2dc176f2d5be7952c47a4e4e8f3b06a90db1c (patch) | |
tree | 770797e88466920af0354358e5707a2397487628 | |
parent | 1b9aba49eab5e85b0d3de8ba630cda6d68546297 (diff) |
cgroup: break kernfs active_ref protection in cgroup directory operations
cgroup_tree_mutex should nest above the kernfs active_ref protection;
however, cgroup_create() and cgroup_rename() were grabbing
cgroup_tree_mutex while under kernfs active_ref protection. This has
actualy possibility to lead to deadlocks in case these operations race
against cgroup_rmdir() which invokes kernfs_remove() on directory
kernfs_node while holding cgroup_tree_mutex.
Neither cgroup_create() or cgroup_rename() requires active_ref
protection. The former already has enough synchronization through
cgroup_lock_live_group() and the latter doesn't care, so this can be
fixed by updating both functions to break all active_ref protections
before grabbing cgroup_tree_mutex.
While this patch fixes the immediate issue, it probably needs further
work in the long term - kernfs directories should enable lockdep
annotations and maybe the better way to handle this is marking
directory nodes as not needing active_ref protection rather than
breaking it in each operation.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r-- | kernel/cgroup.c | 27 |
1 files changed, 26 insertions, 1 deletions
diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 98a8045e2149..58c67b3060b5 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c | |||
@@ -2324,6 +2324,14 @@ static int cgroup_rename(struct kernfs_node *kn, struct kernfs_node *new_parent, | |||
2324 | if (cgroup_sane_behavior(cgrp)) | 2324 | if (cgroup_sane_behavior(cgrp)) |
2325 | return -EPERM; | 2325 | return -EPERM; |
2326 | 2326 | ||
2327 | /* | ||
2328 | * We're gonna grab cgroup_tree_mutex which nests outside kernfs | ||
2329 | * active_ref. kernfs_rename() doesn't require active_ref | ||
2330 | * protection. Break them before grabbing cgroup_tree_mutex. | ||
2331 | */ | ||
2332 | kernfs_break_active_protection(new_parent); | ||
2333 | kernfs_break_active_protection(kn); | ||
2334 | |||
2327 | mutex_lock(&cgroup_tree_mutex); | 2335 | mutex_lock(&cgroup_tree_mutex); |
2328 | mutex_lock(&cgroup_mutex); | 2336 | mutex_lock(&cgroup_mutex); |
2329 | 2337 | ||
@@ -2331,6 +2339,9 @@ static int cgroup_rename(struct kernfs_node *kn, struct kernfs_node *new_parent, | |||
2331 | 2339 | ||
2332 | mutex_unlock(&cgroup_mutex); | 2340 | mutex_unlock(&cgroup_mutex); |
2333 | mutex_unlock(&cgroup_tree_mutex); | 2341 | mutex_unlock(&cgroup_tree_mutex); |
2342 | |||
2343 | kernfs_unbreak_active_protection(kn); | ||
2344 | kernfs_unbreak_active_protection(new_parent); | ||
2334 | return ret; | 2345 | return ret; |
2335 | } | 2346 | } |
2336 | 2347 | ||
@@ -3778,8 +3789,22 @@ static int cgroup_mkdir(struct kernfs_node *parent_kn, const char *name, | |||
3778 | umode_t mode) | 3789 | umode_t mode) |
3779 | { | 3790 | { |
3780 | struct cgroup *parent = parent_kn->priv; | 3791 | struct cgroup *parent = parent_kn->priv; |
3792 | int ret; | ||
3781 | 3793 | ||
3782 | return cgroup_create(parent, name, mode); | 3794 | /* |
3795 | * cgroup_create() grabs cgroup_tree_mutex which nests outside | ||
3796 | * kernfs active_ref and cgroup_create() already synchronizes | ||
3797 | * properly against removal through cgroup_lock_live_group(). | ||
3798 | * Break it before calling cgroup_create(). | ||
3799 | */ | ||
3800 | cgroup_get(parent); | ||
3801 | kernfs_break_active_protection(parent_kn); | ||
3802 | |||
3803 | ret = cgroup_create(parent, name, mode); | ||
3804 | |||
3805 | kernfs_unbreak_active_protection(parent_kn); | ||
3806 | cgroup_put(parent); | ||
3807 | return ret; | ||
3783 | } | 3808 | } |
3784 | 3809 | ||
3785 | /* | 3810 | /* |