diff --git hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c index 474900b3c6c..e30f668a955 100644 --- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c +++ hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c @@ -165,10 +165,12 @@ static int change_effective_user(uid_t user, gid_t group) { * cgroup_file: Path to cgroup file where pid needs to be written to. */ static int write_pid_to_cgroup_as_root(const char* cgroup_file, pid_t pid) { + int rc = 0; uid_t user = geteuid(); gid_t group = getegid(); if (change_effective_user(0, 0) != 0) { - return -1; + rc = -1; + goto cleanup; } // open @@ -176,7 +178,8 @@ static int write_pid_to_cgroup_as_root(const char* cgroup_file, pid_t pid) { if (cgroup_fd == -1) { fprintf(LOGFILE, "Can't open file %s as node manager - %s\n", cgroup_file, strerror(errno)); - return -1; + rc = -1; + goto cleanup; } // write pid @@ -187,15 +190,17 @@ static int write_pid_to_cgroup_as_root(const char* cgroup_file, pid_t pid) { if (written == -1) { fprintf(LOGFILE, "Failed to write pid to file %s - %s\n", cgroup_file, strerror(errno)); - return -1; + rc = -1; + goto cleanup; } +cleanup: // Revert back to the calling user. if (change_effective_user(user, group)) { - return -1; + rc = -1; } - return 0; + return rc; } #endif @@ -204,21 +209,24 @@ static int write_pid_to_cgroup_as_root(const char* cgroup_file, pid_t pid) { * pid_file: Path to pid file where pid needs to be written to */ static int write_pid_to_file_as_nm(const char* pid_file, pid_t pid) { + char *temp_pid_file = NULL; + int rc = 0; uid_t user = geteuid(); gid_t group = getegid(); if (change_effective_user(nm_uid, nm_gid) != 0) { - return -1; + rc = -1; + goto cleanup; } - char *temp_pid_file = concatenate("%s.tmp", "pid_file_path", 1, pid_file); + temp_pid_file = concatenate("%s.tmp", "pid_file_path", 1, pid_file); // create with 700 int pid_fd = open(temp_pid_file, O_WRONLY|O_CREAT|O_EXCL, S_IRWXU); if (pid_fd == -1) { fprintf(LOGFILE, "Can't open file %s as node manager - %s\n", temp_pid_file, strerror(errno)); - free(temp_pid_file); - return -1; + rc = -1; + goto cleanup; } // write pid to temp file @@ -229,8 +237,8 @@ static int write_pid_to_file_as_nm(const char* pid_file, pid_t pid) { if (written == -1) { fprintf(LOGFILE, "Failed to write pid to file %s as node manager - %s\n", temp_pid_file, strerror(errno)); - free(temp_pid_file); - return -1; + rc = -1; + goto cleanup; } // rename temp file to actual pid file @@ -239,29 +247,39 @@ static int write_pid_to_file_as_nm(const char* pid_file, pid_t pid) { fprintf(LOGFILE, "Can't move pid file from %s to %s as node manager - %s\n", temp_pid_file, pid_file, strerror(errno)); unlink(temp_pid_file); - free(temp_pid_file); - return -1; + rc = -1; + goto cleanup; } +cleanup: // Revert back to the calling user. if (change_effective_user(user, group)) { - free(temp_pid_file); - return -1; + rc = -1; } free(temp_pid_file); - return 0; + return rc; } /** * Write the exit code of the container into the exit code file * exit_code_file: Path to exit code file where exit code needs to be written */ -static int write_exit_code_file(const char* exit_code_file, int exit_code) { - char *tmp_ecode_file = concatenate("%s.tmp", "exit_code_path", 1, +static int write_exit_code_file_as_nm(const char* exit_code_file, int exit_code) { + char *tmp_ecode_file = NULL; + int rc = 0; + uid_t user = geteuid(); + gid_t group = getegid(); + if (change_effective_user(nm_uid, nm_gid) != 0) { + rc = -1; + goto cleanup; + } + + tmp_ecode_file = concatenate("%s.tmp", "exit_code_path", 1, exit_code_file); if (tmp_ecode_file == NULL) { - return -1; + rc = -1; + goto cleanup; } // create with 700 @@ -269,8 +287,8 @@ static int write_exit_code_file(const char* exit_code_file, int exit_code) { if (ecode_fd == -1) { fprintf(LOGFILE, "Can't open file %s - %s\n", tmp_ecode_file, strerror(errno)); - free(tmp_ecode_file); - return -1; + rc = -1; + goto cleanup; } char ecode_buf[21]; @@ -280,8 +298,8 @@ static int write_exit_code_file(const char* exit_code_file, int exit_code) { if (written == -1) { fprintf(LOGFILE, "Failed to write exit code to file %s - %s\n", tmp_ecode_file, strerror(errno)); - free(tmp_ecode_file); - return -1; + rc = -1; + goto cleanup; } // rename temp file to actual exit code file @@ -290,12 +308,17 @@ static int write_exit_code_file(const char* exit_code_file, int exit_code) { fprintf(LOGFILE, "Can't move exit code file from %s to %s - %s\n", tmp_ecode_file, exit_code_file, strerror(errno)); unlink(tmp_ecode_file); - free(tmp_ecode_file); - return -1; + rc = -1; + goto cleanup; } +cleanup: + // Revert back to the calling user. + if (change_effective_user(user, group)) { + rc = -1; + } free(tmp_ecode_file); - return 0; + return rc; } /** @@ -308,9 +331,6 @@ static int wait_and_write_exit_code(pid_t pid, const char* exit_code_file) { int exit_code = -1; int waitpid_result; - if (change_effective_user(nm_uid, nm_gid) != 0) { - return -1; - } do { waitpid_result = waitpid(pid, &child_status, 0); } while (waitpid_result == -1 && errno == EINTR); @@ -326,7 +346,7 @@ static int wait_and_write_exit_code(pid_t pid, const char* exit_code_file) { } else { fprintf(LOGFILE, "Unable to determine exit status for pid %d\n", pid); } - if (write_exit_code_file(exit_code_file, exit_code) < 0) { + if (write_exit_code_file_as_nm(exit_code_file, exit_code) < 0) { return -1; } return exit_code; diff --git hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c index a8cc04171ea..9f31afbb065 100644 --- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c +++ hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c @@ -270,8 +270,9 @@ void test_check_user(int expectedFailure) { void test_resolve_config_path() { printf("\nTesting resolve_config_path\n"); - if (strcmp(resolve_config_path("/bin/ls", NULL), "/bin/ls") != 0) { - printf("FAIL: failed to resolve config_name on an absolute path name: /bin/ls\n"); + if (strcmp(resolve_config_path(TEST_ROOT, NULL), TEST_ROOT) != 0) { + printf("FAIL: failed to resolve config_name on an absolute path name: %s\n", + TEST_ROOT); exit(1); } if (strcmp(resolve_config_path(RELTMPDIR TEST_ROOT, TEST_ROOT), TEST_ROOT) != 0) {