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