Commit e7b20d97 authored by Tejun Heo's avatar Tejun Heo

cgroup: Restructure release_agent_path handling

cgrp->root->release_agent_path is protected by both cgroup_mutex and
release_agent_path_lock and readers can hold either one. The
dual-locking scheme was introduced while breaking a locking dependency
issue around cgroup_mutex but doesn't make sense anymore given that
the only remaining reader which uses cgroup_mutex is
cgroup1_releaes_agent().

This patch updates cgroup1_release_agent() to use
release_agent_path_lock so that release_agent_path is always protected
only by release_agent_path_lock.

While at it, convert strlen() based empty string checks to direct
tests on the first character as suggested by Linus.
Signed-off-by: default avatarTejun Heo <tj@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
parent a09833f7
...@@ -38,10 +38,7 @@ static bool cgroup_no_v1_named; ...@@ -38,10 +38,7 @@ static bool cgroup_no_v1_named;
*/ */
static struct workqueue_struct *cgroup_pidlist_destroy_wq; static struct workqueue_struct *cgroup_pidlist_destroy_wq;
/* /* protects cgroup_subsys->release_agent_path */
* Protects cgroup_subsys->release_agent_path. Modifying it also requires
* cgroup_mutex. Reading requires either cgroup_mutex or this spinlock.
*/
static DEFINE_SPINLOCK(release_agent_path_lock); static DEFINE_SPINLOCK(release_agent_path_lock);
bool cgroup1_ssid_disabled(int ssid) bool cgroup1_ssid_disabled(int ssid)
...@@ -775,22 +772,29 @@ void cgroup1_release_agent(struct work_struct *work) ...@@ -775,22 +772,29 @@ void cgroup1_release_agent(struct work_struct *work)
{ {
struct cgroup *cgrp = struct cgroup *cgrp =
container_of(work, struct cgroup, release_agent_work); container_of(work, struct cgroup, release_agent_work);
char *pathbuf = NULL, *agentbuf = NULL; char *pathbuf, *agentbuf;
char *argv[3], *envp[3]; char *argv[3], *envp[3];
int ret; int ret;
mutex_lock(&cgroup_mutex); /* snoop agent path and exit early if empty */
if (!cgrp->root->release_agent_path[0])
return;
/* prepare argument buffers */
pathbuf = kmalloc(PATH_MAX, GFP_KERNEL); pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
agentbuf = kstrdup(cgrp->root->release_agent_path, GFP_KERNEL); agentbuf = kmalloc(PATH_MAX, GFP_KERNEL);
if (!pathbuf || !agentbuf || !strlen(agentbuf)) if (!pathbuf || !agentbuf)
goto out; goto out_free;
spin_lock_irq(&css_set_lock); spin_lock(&release_agent_path_lock);
ret = cgroup_path_ns_locked(cgrp, pathbuf, PATH_MAX, &init_cgroup_ns); strlcpy(agentbuf, cgrp->root->release_agent_path, PATH_MAX);
spin_unlock_irq(&css_set_lock); spin_unlock(&release_agent_path_lock);
if (!agentbuf[0])
goto out_free;
ret = cgroup_path_ns(cgrp, pathbuf, PATH_MAX, &init_cgroup_ns);
if (ret < 0 || ret >= PATH_MAX) if (ret < 0 || ret >= PATH_MAX)
goto out; goto out_free;
argv[0] = agentbuf; argv[0] = agentbuf;
argv[1] = pathbuf; argv[1] = pathbuf;
...@@ -801,11 +805,7 @@ void cgroup1_release_agent(struct work_struct *work) ...@@ -801,11 +805,7 @@ void cgroup1_release_agent(struct work_struct *work)
envp[1] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin"; envp[1] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
envp[2] = NULL; envp[2] = NULL;
mutex_unlock(&cgroup_mutex);
call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
goto out_free;
out:
mutex_unlock(&cgroup_mutex);
out_free: out_free:
kfree(agentbuf); kfree(agentbuf);
kfree(pathbuf); kfree(pathbuf);
......
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