Commit 32a8cf235e2f192eb002755076994525cdbaa35a

Authored by Daniel Lezcano
Committed by Linus Torvalds
1 parent 97978e6d1f

cgroup: make the mount options parsing more accurate

Current behavior:
=================

(1) When we mount a cgroup, we can specify the 'all' option which
    means to enable all the cgroup subsystems.  This is the default option
    when no option is specified.

(2) If we want to mount a cgroup with a subset of the supported cgroup
    subsystems, we have to specify a subsystems name list for the mount
    option.

(3) If we specify another option like 'noprefix' or 'release_agent',
    the actual code wants the 'all' or a subsystem name option specified
    also.  Not critical but a bit not friendly as we should assume (1) in
    this case.

(4) Logically, the 'all' option is mutually exclusive with a subsystem
    name, but this is not detected.

In other words:
 succeed : mount -t cgroup -o all,freezer cgroup /cgroup
	=> is it 'all' or 'freezer' ?
 fails : mount -t cgroup -o noprefix cgroup /cgroup
	=> succeed if we do '-o noprefix,all'

The following patches consolidate a bit the mount options check.

New behavior:
=============

(1) untouched
(2) untouched
(3) the 'all' option will be by default when specifying other than
    a subsystem name option
(4) raises an error

In other words:
 fails   : mount -t cgroup -o all,freezer cgroup /cgroup
 succeed : mount -t cgroup -o noprefix cgroup /cgroup

For the sake of lisibility, the if ... then ... else ... if ...
indentation when parsing the options has been changed to:
if ... then
	...
	continue
fi

Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com>
Reviewed-by: Li Zefan <lizf@cn.fujitsu.com>
Reviewed-by: Paul Menage <menage@google.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jamal Hadi Salim <hadi@cyberus.ca>
Cc: Matt Helsley <matthltc@us.ibm.com>
Cc: Balbir Singh <balbir@linux.vnet.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Showing 1 changed file with 60 additions and 30 deletions Side-by-side Diff

... ... @@ -1074,7 +1074,8 @@
1074 1074 */
1075 1075 static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
1076 1076 {
1077   - char *token, *o = data ?: "all";
  1077 + char *token, *o = data;
  1078 + bool all_ss = false, one_ss = false;
1078 1079 unsigned long mask = (unsigned long)-1;
1079 1080 int i;
1080 1081 bool module_pin_failed = false;
1081 1082  
1082 1083  
1083 1084  
... ... @@ -1090,24 +1091,27 @@
1090 1091 while ((token = strsep(&o, ",")) != NULL) {
1091 1092 if (!*token)
1092 1093 return -EINVAL;
1093   - if (!strcmp(token, "all")) {
1094   - /* Add all non-disabled subsystems */
1095   - opts->subsys_bits = 0;
1096   - for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
1097   - struct cgroup_subsys *ss = subsys[i];
1098   - if (ss == NULL)
1099   - continue;
1100   - if (!ss->disabled)
1101   - opts->subsys_bits |= 1ul << i;
1102   - }
1103   - } else if (!strcmp(token, "none")) {
  1094 + if (!strcmp(token, "none")) {
1104 1095 /* Explicitly have no subsystems */
1105 1096 opts->none = true;
1106   - } else if (!strcmp(token, "noprefix")) {
  1097 + continue;
  1098 + }
  1099 + if (!strcmp(token, "all")) {
  1100 + /* Mutually exclusive option 'all' + subsystem name */
  1101 + if (one_ss)
  1102 + return -EINVAL;
  1103 + all_ss = true;
  1104 + continue;
  1105 + }
  1106 + if (!strcmp(token, "noprefix")) {
1107 1107 set_bit(ROOT_NOPREFIX, &opts->flags);
1108   - } else if (!strcmp(token, "clone_children")) {
  1108 + continue;
  1109 + }
  1110 + if (!strcmp(token, "clone_children")) {
1109 1111 opts->clone_children = true;
1110   - } else if (!strncmp(token, "release_agent=", 14)) {
  1112 + continue;
  1113 + }
  1114 + if (!strncmp(token, "release_agent=", 14)) {
1111 1115 /* Specifying two release agents is forbidden */
1112 1116 if (opts->release_agent)
1113 1117 return -EINVAL;
... ... @@ -1115,7 +1119,9 @@
1115 1119 kstrndup(token + 14, PATH_MAX - 1, GFP_KERNEL);
1116 1120 if (!opts->release_agent)
1117 1121 return -ENOMEM;
1118   - } else if (!strncmp(token, "name=", 5)) {
  1122 + continue;
  1123 + }
  1124 + if (!strncmp(token, "name=", 5)) {
1119 1125 const char *name = token + 5;
1120 1126 /* Can't specify an empty name */
1121 1127 if (!strlen(name))
... ... @@ -1137,20 +1143,44 @@
1137 1143 GFP_KERNEL);
1138 1144 if (!opts->name)
1139 1145 return -ENOMEM;
1140   - } else {
1141   - struct cgroup_subsys *ss;
1142   - for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
1143   - ss = subsys[i];
1144   - if (ss == NULL)
1145   - continue;
1146   - if (!strcmp(token, ss->name)) {
1147   - if (!ss->disabled)
1148   - set_bit(i, &opts->subsys_bits);
1149   - break;
1150   - }
1151   - }
1152   - if (i == CGROUP_SUBSYS_COUNT)
1153   - return -ENOENT;
  1146 +
  1147 + continue;
  1148 + }
  1149 +
  1150 + for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
  1151 + struct cgroup_subsys *ss = subsys[i];
  1152 + if (ss == NULL)
  1153 + continue;
  1154 + if (strcmp(token, ss->name))
  1155 + continue;
  1156 + if (ss->disabled)
  1157 + continue;
  1158 +
  1159 + /* Mutually exclusive option 'all' + subsystem name */
  1160 + if (all_ss)
  1161 + return -EINVAL;
  1162 + set_bit(i, &opts->subsys_bits);
  1163 + one_ss = true;
  1164 +
  1165 + break;
  1166 + }
  1167 + if (i == CGROUP_SUBSYS_COUNT)
  1168 + return -ENOENT;
  1169 + }
  1170 +
  1171 + /*
  1172 + * If the 'all' option was specified select all the subsystems,
  1173 + * otherwise 'all, 'none' and a subsystem name options were not
  1174 + * specified, let's default to 'all'
  1175 + */
  1176 + if (all_ss || (!all_ss && !one_ss && !opts->none)) {
  1177 + for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
  1178 + struct cgroup_subsys *ss = subsys[i];
  1179 + if (ss == NULL)
  1180 + continue;
  1181 + if (ss->disabled)
  1182 + continue;
  1183 + set_bit(i, &opts->subsys_bits);
1154 1184 }
1155 1185 }
1156 1186