linux-kernel - [PATCH 4/4] cgroup: move the one-off opts sanity check in cgroup_root_from_opts() to parse_cgroupfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 28 Jan 2014 10:32:05 -0500
From:	Tejun Heo <tj@...nel.org>
To:	lizefan@...wei.com
Cc:	containers@...ts.linux-foundation.org, cgroups@...r.kernel.org,
	linux-kernel@...r.kernel.org, Tejun Heo <tj@...nel.org>
Subject: [PATCH 4/4] cgroup: move the one-off opts sanity check in cgroup_root_from_opts() to parse_cgroupfs_options()

cgroup_root_from_opts() checks whether (!opts->subsys_mask &&
!opts->none) and returns NULL if so.  After that, if allocation fails,
returns ERR_PTR(-ENOMEM).  The caller, cgroup_mount(), doesn't treat
NULL as an error but set opts.new_root to NULL; however, later on,
cgroup_set_super() fails with -EINVAL if new_root is NULL.

This is one bizarre error handling sequence especially when all other
opts sanity checks including the very close (!opts->subsys_mask &&
!opts->name) check are done in parse_cgroupfs_options().

Let's move the one-off check in cgroup_root_from_opts() to
parse_cgroupfs_options() where it can be combined with the
(!opts->subsys_mask && !opts->name) check.  cgroup_root_from_opts() is
updated to return NULL on memory allocation failure.

Signed-off-by: Tejun Heo <tj@...nel.org>
---
 kernel/cgroup.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index b0e030f..5c8fe40 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1265,10 +1265,10 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
 		return -EINVAL;
 
 	/*
-	 * We either have to specify by name or by subsystems. (So all
-	 * empty hierarchies must have a name).
+	 * We either have to specify by name or by subsystems.  All empty
+	 * hierarchies must have a name and also "none" option specified.
 	 */
-	if (!opts->subsys_mask && !opts->name)
+	if (!opts->subsys_mask && (!opts->name || !opts->none))
 		return -EINVAL;
 
 	return 0;
@@ -1417,12 +1417,9 @@ static struct cgroupfs_root *cgroup_root_from_opts(struct cgroup_sb_opts *opts)
 {
 	struct cgroupfs_root *root;
 
-	if (!opts->subsys_mask && !opts->none)
-		return NULL;
-
 	root = kzalloc(sizeof(*root), GFP_KERNEL);
 	if (!root)
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 
 	init_cgroup_root(root);
 
@@ -1461,10 +1458,6 @@ static int cgroup_set_super(struct super_block *sb, void *data)
 	int ret;
 	struct cgroup_sb_opts *opts = data;
 
-	/* If we don't have a new root, we can't set up a new sb */
-	if (!opts->new_root)
-		return -EINVAL;
-
 	BUG_ON(!opts->subsys_mask && !opts->none);
 
 	ret = set_anon_super(sb, NULL);
@@ -1532,8 +1525,8 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type,
 	 * reusing an existing hierarchy.
 	 */
 	new_root = cgroup_root_from_opts(&opts);
-	if (IS_ERR(new_root)) {
-		ret = PTR_ERR(new_root);
+	if (!new_root) {
+		ret = -ENOMEM;
 		goto out_err;
 	}
 	opts.new_root = new_root;
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/