From e20a840174bc2b27fcc0935e0086977bd6fbfcb3 Mon Sep 17 00:00:00 2001 From: Robert Kanter Date: Thu, 7 Jun 2018 17:09:34 -0700 Subject: [PATCH] Disable mounting cgroups by default (miklos.szegedi@cloudera.com via rkanter) (cherry picked from commit 351cf87c92872d90f62c476f85ae4d02e485769c) (cherry picked from commit d61d84279f7f22867c23dd95e8bfeb70ea7e0690) (cherry picked from commit f5fd5aa025c904e9a2ff8c5fd932aaed2363a6a0) --- .../impl/container-executor.c | 54 +++++++++++++------ .../impl/container-executor.h | 4 ++ .../native/container-executor/impl/main.c | 19 ++++--- .../src/site/markdown/NodeManagerCgroups.md | 2 +- 4 files changed, 55 insertions(+), 24 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c index a5a7e6ab0f5bb..b2c2bab711354 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c @@ -70,6 +70,7 @@ static const char* DEFAULT_BANNED_USERS[] = {"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; //location of traffic control binary static const char* TC_BIN = "/sbin/tc"; @@ -469,6 +470,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. */ @@ -2129,20 +2136,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); } @@ -2166,11 +2178,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) { @@ -2178,13 +2195,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)); @@ -2193,13 +2207,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; @@ -2207,6 +2228,7 @@ int mount_cgroup(const char *pair, const char *hierarchy) { } } +cleanup: free(controller); free(mount_path); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h index f81447ced168b..5cb6b07d07b0b 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h @@ -61,6 +61,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" /* Macros for min/max. */ @@ -240,6 +241,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 */ diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c index b913f166c6787..58ffc91aa7a5b 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c @@ -246,14 +246,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) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/NodeManagerCgroups.md b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/NodeManagerCgroups.md index d36280113cfcd..4a83dcea79b96 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-site/src/site/markdown/NodeManagerCgroups.md +++ b/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 `/cpu,cpuacct`. Valid subsystem names are `cpu, cpuacct, cpuset, memory, net_cls, blkio, freezer, devices`.| CGroups and security