Commit e788e066c651b1bbf4a927dc95395c1aa13be436
Committed by
Linus Torvalds
1 parent
db3b14978a
Exists in
master
and in
4 other branches
cgroup files: move the release_agent file to use typed handlers
Adds cgroup_release_agent_write() and cgroup_release_agent_show() methods to handle writing/reading the path to a cgroup hierarchy's release agent. As a result, cgroup_common_file_read() is now unnecessary. As part of the change, a previously-tolerated race in cgroup_release_agent() is avoided by copying the current release_agent_path prior to calling call_usermode_helper(). Signed-off-by: Paul Menage <menage@google.com> Cc: Paul Jackson <pj@sgi.com> Cc: Pavel Emelyanov <xemul@openvz.org> Cc: Balbir Singh <balbir@in.ibm.com> Acked-by: Serge Hallyn <serue@us.ibm.com> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Showing 2 changed files with 59 additions and 68 deletions Side-by-side Diff
include/linux/cgroup.h
... | ... | @@ -295,6 +295,8 @@ |
295 | 295 | |
296 | 296 | int cgroup_is_removed(const struct cgroup *cgrp); |
297 | 297 | |
298 | +int cgroup_lock_live_group(struct cgroup *cgrp); | |
299 | + | |
298 | 300 | int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen); |
299 | 301 | |
300 | 302 | int cgroup_task_count(const struct cgroup *cgrp); |
kernel/cgroup.c
... | ... | @@ -89,11 +89,7 @@ |
89 | 89 | /* Hierarchy-specific flags */ |
90 | 90 | unsigned long flags; |
91 | 91 | |
92 | - /* The path to use for release notifications. No locking | |
93 | - * between setting and use - so if userspace updates this | |
94 | - * while child cgroups exist, you could miss a | |
95 | - * notification. We ensure that it's always a valid | |
96 | - * NUL-terminated string */ | |
92 | + /* The path to use for release notifications. */ | |
97 | 93 | char release_agent_path[PATH_MAX]; |
98 | 94 | }; |
99 | 95 | |
... | ... | @@ -1329,6 +1325,45 @@ |
1329 | 1325 | FILE_RELEASE_AGENT, |
1330 | 1326 | }; |
1331 | 1327 | |
1328 | +/** | |
1329 | + * cgroup_lock_live_group - take cgroup_mutex and check that cgrp is alive. | |
1330 | + * @cgrp: the cgroup to be checked for liveness | |
1331 | + * | |
1332 | + * Returns true (with lock held) on success, or false (with no lock | |
1333 | + * held) on failure. | |
1334 | + */ | |
1335 | +int cgroup_lock_live_group(struct cgroup *cgrp) | |
1336 | +{ | |
1337 | + mutex_lock(&cgroup_mutex); | |
1338 | + if (cgroup_is_removed(cgrp)) { | |
1339 | + mutex_unlock(&cgroup_mutex); | |
1340 | + return false; | |
1341 | + } | |
1342 | + return true; | |
1343 | +} | |
1344 | + | |
1345 | +static int cgroup_release_agent_write(struct cgroup *cgrp, struct cftype *cft, | |
1346 | + const char *buffer) | |
1347 | +{ | |
1348 | + BUILD_BUG_ON(sizeof(cgrp->root->release_agent_path) < PATH_MAX); | |
1349 | + if (!cgroup_lock_live_group(cgrp)) | |
1350 | + return -ENODEV; | |
1351 | + strcpy(cgrp->root->release_agent_path, buffer); | |
1352 | + mutex_unlock(&cgroup_mutex); | |
1353 | + return 0; | |
1354 | +} | |
1355 | + | |
1356 | +static int cgroup_release_agent_show(struct cgroup *cgrp, struct cftype *cft, | |
1357 | + struct seq_file *seq) | |
1358 | +{ | |
1359 | + if (!cgroup_lock_live_group(cgrp)) | |
1360 | + return -ENODEV; | |
1361 | + seq_puts(seq, cgrp->root->release_agent_path); | |
1362 | + seq_putc(seq, '\n'); | |
1363 | + mutex_unlock(&cgroup_mutex); | |
1364 | + return 0; | |
1365 | +} | |
1366 | + | |
1332 | 1367 | static ssize_t cgroup_write_X64(struct cgroup *cgrp, struct cftype *cft, |
1333 | 1368 | struct file *file, |
1334 | 1369 | const char __user *userbuf, |
... | ... | @@ -1443,10 +1478,6 @@ |
1443 | 1478 | else |
1444 | 1479 | clear_bit(CGRP_NOTIFY_ON_RELEASE, &cgrp->flags); |
1445 | 1480 | break; |
1446 | - case FILE_RELEASE_AGENT: | |
1447 | - BUILD_BUG_ON(sizeof(cgrp->root->release_agent_path) < PATH_MAX); | |
1448 | - strcpy(cgrp->root->release_agent_path, buffer); | |
1449 | - break; | |
1450 | 1481 | default: |
1451 | 1482 | retval = -EINVAL; |
1452 | 1483 | goto out2; |
... | ... | @@ -1506,49 +1537,6 @@ |
1506 | 1537 | return simple_read_from_buffer(buf, nbytes, ppos, tmp, len); |
1507 | 1538 | } |
1508 | 1539 | |
1509 | -static ssize_t cgroup_common_file_read(struct cgroup *cgrp, | |
1510 | - struct cftype *cft, | |
1511 | - struct file *file, | |
1512 | - char __user *buf, | |
1513 | - size_t nbytes, loff_t *ppos) | |
1514 | -{ | |
1515 | - enum cgroup_filetype type = cft->private; | |
1516 | - char *page; | |
1517 | - ssize_t retval = 0; | |
1518 | - char *s; | |
1519 | - | |
1520 | - if (!(page = (char *)__get_free_page(GFP_KERNEL))) | |
1521 | - return -ENOMEM; | |
1522 | - | |
1523 | - s = page; | |
1524 | - | |
1525 | - switch (type) { | |
1526 | - case FILE_RELEASE_AGENT: | |
1527 | - { | |
1528 | - struct cgroupfs_root *root; | |
1529 | - size_t n; | |
1530 | - mutex_lock(&cgroup_mutex); | |
1531 | - root = cgrp->root; | |
1532 | - n = strnlen(root->release_agent_path, | |
1533 | - sizeof(root->release_agent_path)); | |
1534 | - n = min(n, (size_t) PAGE_SIZE); | |
1535 | - strncpy(s, root->release_agent_path, n); | |
1536 | - mutex_unlock(&cgroup_mutex); | |
1537 | - s += n; | |
1538 | - break; | |
1539 | - } | |
1540 | - default: | |
1541 | - retval = -EINVAL; | |
1542 | - goto out; | |
1543 | - } | |
1544 | - *s++ = '\n'; | |
1545 | - | |
1546 | - retval = simple_read_from_buffer(buf, nbytes, ppos, page, s - page); | |
1547 | -out: | |
1548 | - free_page((unsigned long)page); | |
1549 | - return retval; | |
1550 | -} | |
1551 | - | |
1552 | 1540 | static ssize_t cgroup_file_read(struct file *file, char __user *buf, |
1553 | 1541 | size_t nbytes, loff_t *ppos) |
1554 | 1542 | { |
... | ... | @@ -1606,6 +1594,7 @@ |
1606 | 1594 | |
1607 | 1595 | static struct file_operations cgroup_seqfile_operations = { |
1608 | 1596 | .read = seq_read, |
1597 | + .write = cgroup_file_write, | |
1609 | 1598 | .llseek = seq_lseek, |
1610 | 1599 | .release = cgroup_seqfile_release, |
1611 | 1600 | }; |
... | ... | @@ -2283,8 +2272,9 @@ |
2283 | 2272 | |
2284 | 2273 | static struct cftype cft_release_agent = { |
2285 | 2274 | .name = "release_agent", |
2286 | - .read = cgroup_common_file_read, | |
2287 | - .write = cgroup_common_file_write, | |
2275 | + .read_seq_string = cgroup_release_agent_show, | |
2276 | + .write_string = cgroup_release_agent_write, | |
2277 | + .max_write_len = PATH_MAX, | |
2288 | 2278 | .private = FILE_RELEASE_AGENT, |
2289 | 2279 | }; |
2290 | 2280 | |
2291 | 2281 | |
2292 | 2282 | |
2293 | 2283 | |
... | ... | @@ -3111,27 +3101,24 @@ |
3111 | 3101 | while (!list_empty(&release_list)) { |
3112 | 3102 | char *argv[3], *envp[3]; |
3113 | 3103 | int i; |
3114 | - char *pathbuf; | |
3104 | + char *pathbuf = NULL, *agentbuf = NULL; | |
3115 | 3105 | struct cgroup *cgrp = list_entry(release_list.next, |
3116 | 3106 | struct cgroup, |
3117 | 3107 | release_list); |
3118 | 3108 | list_del_init(&cgrp->release_list); |
3119 | 3109 | spin_unlock(&release_list_lock); |
3120 | 3110 | pathbuf = kmalloc(PAGE_SIZE, GFP_KERNEL); |
3121 | - if (!pathbuf) { | |
3122 | - spin_lock(&release_list_lock); | |
3123 | - continue; | |
3124 | - } | |
3111 | + if (!pathbuf) | |
3112 | + goto continue_free; | |
3113 | + if (cgroup_path(cgrp, pathbuf, PAGE_SIZE) < 0) | |
3114 | + goto continue_free; | |
3115 | + agentbuf = kstrdup(cgrp->root->release_agent_path, GFP_KERNEL); | |
3116 | + if (!agentbuf) | |
3117 | + goto continue_free; | |
3125 | 3118 | |
3126 | - if (cgroup_path(cgrp, pathbuf, PAGE_SIZE) < 0) { | |
3127 | - kfree(pathbuf); | |
3128 | - spin_lock(&release_list_lock); | |
3129 | - continue; | |
3130 | - } | |
3131 | - | |
3132 | 3119 | i = 0; |
3133 | - argv[i++] = cgrp->root->release_agent_path; | |
3134 | - argv[i++] = (char *)pathbuf; | |
3120 | + argv[i++] = agentbuf; | |
3121 | + argv[i++] = pathbuf; | |
3135 | 3122 | argv[i] = NULL; |
3136 | 3123 | |
3137 | 3124 | i = 0; |
3138 | 3125 | |
... | ... | @@ -3145,8 +3132,10 @@ |
3145 | 3132 | * be a slow process */ |
3146 | 3133 | mutex_unlock(&cgroup_mutex); |
3147 | 3134 | call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC); |
3148 | - kfree(pathbuf); | |
3149 | 3135 | mutex_lock(&cgroup_mutex); |
3136 | + continue_free: | |
3137 | + kfree(pathbuf); | |
3138 | + kfree(agentbuf); | |
3150 | 3139 | spin_lock(&release_list_lock); |
3151 | 3140 | } |
3152 | 3141 | spin_unlock(&release_list_lock); |