Disable mounting cgroups by default (miklos.szegedi@cloudera.com via rkanter)

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c

@@ -73,6 +73,7 @@ static const char* DEFAULT_BANNED_USERS[] = {"yarn", "mapred", "hdfs", "bin", 0}
 
 static const int DEFAULT_DOCKER_SUPPORT_ENABLED = 0;
 static const int DEFAULT_TC_SUPPORT_ENABLED = 0;
+static const int DEFAULT_MOUNT_CGROUP_SUPPORT_ENABLED = 0;
 
 static const char* PROC_PATH = "/proc";
 
@@ -482,6 +483,12 @@ int is_tc_support_enabled() {
     DEFAULT_TC_SUPPORT_ENABLED, &executor_cfg);
 }
 
+int is_mount_cgroups_support_enabled() {
+    return is_feature_enabled(MOUNT_CGROUP_SUPPORT_ENABLED_KEY,
+                              DEFAULT_MOUNT_CGROUP_SUPPORT_ENABLED,
+                              &executor_cfg);
+}
+
 /**
  * Utility function to concatenate argB to argA using the concat_pattern.
  */
@@ -2346,20 +2353,25 @@ void chown_dir_contents(const char *dir_path, uid_t uid, gid_t gid) {
   DIR *dp;
   struct dirent *ep;
 
-  char *path_tmp = malloc(strlen(dir_path) + NAME_MAX + 2);
+  size_t len = strlen(dir_path) + NAME_MAX + 2;
+  char *path_tmp = malloc(len);
   if (path_tmp == NULL) {
     return;
   }
 
-  char *buf = stpncpy(path_tmp, dir_path, strlen(dir_path));
-  *buf++ = '/';
-
   dp = opendir(dir_path);
   if (dp != NULL) {
     while ((ep = readdir(dp)) != NULL) {
-      stpncpy(buf, ep->d_name, strlen(ep->d_name));
-      buf[strlen(ep->d_name)] = '\0';
-      change_owner(path_tmp, uid, gid);
+      if (strcmp(ep->d_name, ".") != 0 &&
+          strcmp(ep->d_name, "..") != 0 &&
+          strstr(ep->d_name, "..") == NULL) {
+        int result = snprintf(path_tmp, len, "%s/%s", dir_path, ep->d_name);
+        if (result > 0 && result < len) {
+          change_owner(path_tmp, uid, gid);
+        } else {
+          fprintf(LOGFILE, "Ignored %s/%s due to length", dir_path, ep->d_name);
+        }
+      }
     }
     closedir(dp);
   }
@@ -2383,11 +2395,16 @@ int mount_cgroup(const char *pair, const char *hierarchy) {
   char *mount_path = malloc(len);
   char hier_path[EXECUTOR_PATH_MAX];
   int result = 0;
-  struct stat sb;
 
   if (controller == NULL || mount_path == NULL) {
     fprintf(LOGFILE, "Failed to mount cgroup controller; not enough memory\n");
     result = OUT_OF_MEMORY;
+    goto cleanup;
+  }
+  if (hierarchy == NULL || strstr(hierarchy, "..") != NULL) {
+    fprintf(LOGFILE, "Unsupported cgroup hierarhy path detected.\n");
+    result = INVALID_COMMAND_PROVIDED;
+    goto cleanup;
   }
   if (get_kv_key(pair, controller, len) < 0 ||
       get_kv_value(pair, mount_path, len) < 0) {
@@ -2395,13 +2412,10 @@ int mount_cgroup(const char *pair, const char *hierarchy) {
               pair);
     result = -1;
   } else {
-    if (stat(mount_path, &sb) != 0) {
-      // Create mount point, if it does not exist
-      const mode_t mount_perms = S_IRWXU | S_IRGRP | S_IXGRP;
-      if (mkdirs(mount_path, mount_perms) == 0) {
-        fprintf(LOGFILE, "Failed to create cgroup mount point %s at %s\n",
-          controller, mount_path);
-      }
+    if (strstr(mount_path, "..") != NULL) {
+      fprintf(LOGFILE, "Unsupported cgroup mount path detected.\n");
+      result = INVALID_COMMAND_PROVIDED;
+      goto cleanup;
     }
     if (mount("none", mount_path, "cgroup", 0, controller) == 0) {
       char *buf = stpncpy(hier_path, mount_path, strlen(mount_path));
@@ -2410,13 +2424,20 @@ int mount_cgroup(const char *pair, const char *hierarchy) {
 
       // create hierarchy as 0750 and chown to Hadoop NM user
       const mode_t perms = S_IRWXU | S_IRGRP | S_IXGRP;
+      struct stat sb;
+      if (stat(hier_path, &sb) == 0 &&
+          (sb.st_uid != nm_uid || sb.st_gid != nm_gid)) {
+        fprintf(LOGFILE, "cgroup hierarchy %s already owned by another user %d\n", hier_path, sb.st_uid);
+        result = INVALID_COMMAND_PROVIDED;
+        goto cleanup;
+      }
       if (mkdirs(hier_path, perms) == 0) {
         change_owner(hier_path, nm_uid, nm_gid);
         chown_dir_contents(hier_path, nm_uid, nm_gid);
       }
     } else {
       fprintf(LOGFILE, "Failed to mount cgroup controller %s at %s - %s\n",
-                controller, mount_path, strerror(errno));
+              controller, mount_path, strerror(errno));
       // if controller is already mounted, don't stop trying to mount others
       if (errno != EBUSY) {
         result = -1;
@@ -2424,6 +2445,7 @@ int mount_cgroup(const char *pair, const char *hierarchy) {
     }
   }
 
+cleanup:
   free(controller);
   free(mount_path);
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h

@@ -64,6 +64,7 @@ enum operations {
 #define ALLOWED_SYSTEM_USERS_KEY "allowed.system.users"
 #define DOCKER_SUPPORT_ENABLED_KEY "feature.docker.enabled"
 #define TC_SUPPORT_ENABLED_KEY "feature.tc.enabled"
+#define MOUNT_CGROUP_SUPPORT_ENABLED_KEY "feature.mount-cgroup.enabled"
 #define TMP_DIR "tmp"
 
 extern struct passwd *user_detail;
@@ -238,6 +239,9 @@ int is_feature_enabled(const char* feature_key, int default_value,
 /** Check if tc (traffic control) support is enabled in configuration. */
 int is_tc_support_enabled();
 
+/** Check if cgroup mount support is enabled in configuration. */
+int is_mount_cgroups_support_enabled();
+
 /**
  * Run a batch of tc commands that modify interface configuration
  */

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c

@@ -262,14 +262,19 @@ static int validate_arguments(int argc, char **argv , int *operation) {
   }
 
   if (strcmp("--mount-cgroups", argv[1]) == 0) {
-    if (argc < 4) {
-      display_usage(stdout);
-      return INVALID_ARGUMENT_NUMBER;
+    if (is_mount_cgroups_support_enabled()) {
+      if (argc < 4) {
+        display_usage(stdout);
+        return INVALID_ARGUMENT_NUMBER;
+      }
+      optind++;
+      cmd_input.cgroups_hierarchy = argv[optind++];
+      *operation = MOUNT_CGROUPS;
+      return 0;
+    } else {
+      display_feature_disabled_message("mount cgroup");
+      return FEATURE_DISABLED;
     }
-    optind++;
-    cmd_input.cgroups_hierarchy = argv[optind++];
-    *operation = MOUNT_CGROUPS;
-    return 0;
   }
 
   if (strcmp("--tc-modify-state", argv[1]) == 0) {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/NodeManagerCgroups.md

@@ -50,7 +50,7 @@ YARN uses CGroups through a directory structure mounted into the file system by
 | Option | Description |
 |:---- |:---- |
 | Discover CGroups mounted already | This should be used on newer systems like RHEL7 or Ubuntu16 or if the administrator mounts CGroups before YARN starts. Set `yarn.nodemanager.linux-container-executor.cgroups.mount` to false and leave other settings set to their defaults. YARN will locate the mount points in `/proc/mounts`. Common locations include `/sys/fs/cgroup` and `/cgroup`. The default location can vary depending on the Linux distribution in use.|
-| CGroups mounted by YARN | If the system does not have CGroups mounted or it is mounted to an inaccessible location then point `yarn.nodemanager.linux-container-executor.cgroups.mount-path` to an empty directory. Set `yarn.nodemanager.linux-container-executor.cgroups.mount` to true. A point to note here is that the container-executor binary will try to create and mount each subsystem as a subdirectory under this path. If `cpu` is already mounted somewhere with `cpuacct`, then the directory `cpu,cpuacct` will be created for the hierarchy.|
+| CGroups mounted by YARN | IMPORTANT: This option is deprecated due to security reasons with the `container-executor.cfg` option `feature.mount-cgroup.enabled=0` by default. Please mount cgroups before launching YARN.|
 | CGroups mounted already or linked but not in `/proc/mounts` | If cgroups is accessible through lxcfs or simulated by another filesystem, then point `yarn.nodemanager.linux-container-executor.cgroups.mount-path` to your CGroups root directory. Set `yarn.nodemanager.linux-container-executor.cgroups.mount` to false. YARN tries to use this path first, before any CGroup mount point discovery. The path should have a subdirectory for each CGroup hierarchy named by the comma separated CGroup subsystems supported like `<path>/cpu,cpuacct`. Valid subsystem names are `cpu, cpuacct, cpuset, memory, net_cls, blkio, freezer, devices`.|
 
 CGroups and security