In patch 87cb5fdb5b5c77ac617b46a0fe118a7d50a77b1c function
cgroup_show_options started to lock cgroup_mutex, which introduced
new deadlock possibility, described below:
Thread A:
m_start() --> down_read(&namespace_sem);
cgroup_show_options() --> mutex_lock(&cgroup_mutex);
Thread B:
attach_task_by_pid()
cgroup_lock_live_group --> mutex_lock(&cgroup_mutex);
threadgroup_lock() --> down_write(&tsk->signal->group_rwsem);
Thread C:
copy_process
threadgroup_change_begin() --> down_read(&tsk->signal->group_rwsem);
copy_namespaces
create_new_namespaces
copy_mnt_ns
namespace_lock() --> down_write(&namespace_sem)
Clearly cgroup_mutex can not be locked right after locking
namespace_sem, because opposite locking order is also present in
the code. To get rid of cgroup_mutex it's synchonization role should
transition to cgroup_root_mutex, that was created specifically to defeat
the described locking order problem.
In this location cgroup_mutex provided 2 guarantees:
1. Safe call for 'task_cgroup_from_root', that asserts cgroup_mutex
ownership.
2. Safe read access to cgroup->ve_owner
These guarantees are now transferred to cgroup_root_mutex in the
following way:
1. task_cgroup_from_root assertion is modified to take into account
cgroup_root_mutex as one of two possible locks.
2. cgroup->ve_owner field modifications are done with
cgroup_root_mutex also locked.
https://jira.sw.ru/browse/PSBM-121438
Signed-Off-By: Valeriy Vdovin <valeriy.vdo...@virtuozzo.com>
---
kernel/cgroup.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 27d7a5e..db6a5ba 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -669,7 +669,7 @@ static struct cgroup *task_cgroup_from_root(struct
task_struct *task,
struct css_set *css;
struct cgroup *res = NULL;
- BUG_ON(!mutex_is_locked(&cgroup_mutex));
+ BUG_ON(!mutex_is_locked(&cgroup_mutex) &&
!mutex_is_locked(&cgroup_root_mutex));
read_lock(&css_set_lock);
/*
* No need to lock the task - since we hold cgroup_mutex the
@@ -1100,7 +1100,6 @@ static int cgroup_show_options(struct seq_file *seq,
struct dentry *dentry)
struct cgroup_subsys *ss;
struct cgroup *root_cgrp = &root->top_cgroup;
- mutex_lock(&cgroup_mutex);
mutex_lock(&cgroup_root_mutex);
for_each_subsys(root, ss)
seq_printf(seq, ",%s", ss->name);
@@ -1142,7 +1141,6 @@ static int cgroup_show_options(struct seq_file *seq,
struct dentry *dentry)
if (strlen(root->name))
seq_show_option(seq, "name", root->name);
mutex_unlock(&cgroup_root_mutex);
- mutex_unlock(&cgroup_mutex);
return 0;
}
@@ -2529,6 +2527,8 @@ static int cgroup_release_agent_write(struct cgroup
*cgrp, struct cftype *cft,
if (!cgroup_lock_live_group(cgrp))
return -ENODEV;
+ mutex_lock(&cgroup_root_mutex);
+
/*
* Call to cgroup_get_local_root is a way to make sure
* that cgrp in the argument is valid "virtual root"
@@ -2551,6 +2551,7 @@ static int cgroup_release_agent_write(struct cgroup
*cgrp, struct cftype *cft,
ret = -ENODEV;
out:
+ mutex_unlock(&cgroup_root_mutex);
mutex_unlock(&cgroup_mutex);
return ret;
}
--
1.8.3.1
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
