[Devel] [PATCH RH7] cgroup: do not use cgroup_mutex in cgroup_show_options

[Valeriy Vdovin]

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.vdovin at 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