Apache
/
hadoop
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

YARN-4731. container-executor should not follow symlinks in recursive_unlink_children. Contributed by Colin Patrick McCabe 

(cherry picked from commit c58a6d53c5)

Conflicts:
	hadoop-yarn-project/CHANGES.txt
	hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
This commit is contained in:
Jason Lowe 2016-02-29 15:24:35 +00:00 committed by Jason Lowe
parent 293ac4c18b
commit 4fc070ac59
2 changed files with 150 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1811,9 +1811,9 @@ static int rmdir_as_nm(const char* path) {
static int open_helper(int dirfd, const char *name) {
int fd;
if (dirfd >= 0) {
fd = openat(dirfd, name, O_RDONLY);
fd = openat(dirfd, name, O_RDONLY | O_NOFOLLOW);
} else {
fd = open(name, O_RDONLY);
fd = open(name, O_RDONLY | O_NOFOLLOW);
}
if (fd >= 0) {
return fd;
@ -1847,6 +1847,34 @@ static int unlink_helper(int dirfd, const char *name, int flags) {
return errno;
}
/**
* Determine if an entry in a directory is a symlink.
*
* @param dirfd The directory file descriptor, or -1 if there is none.
* @param name If dirfd is -1, this is the path to examine.
* Otherwise, this is the file name in the directory to
* examine.
*
* @return 0 if the entry is not a symlink
* 1 if the entry is a symlink
* A negative errno code if we couldn't access the entry.
*/
static int is_symlink_helper(int dirfd, const char *name)
{
struct stat stat;
if (dirfd < 0) {
if (lstat(name, &stat) < 0) {
return -errno;
}
} else {
if (fstatat(dirfd, name, &stat, AT_SYMLINK_NOFOLLOW) < 0) {
return -errno;
}
}
return !!S_ISLNK(stat.st_mode);
}
static int recursive_unlink_helper(int dirfd, const char *name,
const char* fullpath)
{
@ -1854,6 +1882,28 @@ static int recursive_unlink_helper(int dirfd, const char *name,
DIR *dfd = NULL;
struct stat stat;
// Check to see if the file is a symlink. If so, delete the symlink rather
// than what it points to.
ret = is_symlink_helper(dirfd, name);
if (ret < 0) {
// is_symlink_helper failed.
ret = -ret;
fprintf(LOGFILE, "is_symlink_helper(%s) failed: %s\n",
fullpath, strerror(ret));
goto done;
} else if (ret == 1) {
// is_symlink_helper determined that the path is a symlink.
ret = unlink_helper(dirfd, name, 0);
if (ret) {
fprintf(LOGFILE, "failed to unlink symlink %s: %s\n",
fullpath, strerror(ret));
}
goto done;
}
// Open the file. We use O_NOFOLLOW here to ensure that we if a symlink was
// swapped in by an attacker, we will fail to follow it rather than deleting
// something we potentially should not.
fd = open_helper(dirfd, name);
if (fd == -EACCES) {
ret = chmod_helper(dirfd, name, 0700);

99
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
View File

@ -714,6 +714,100 @@ void test_run_container() {
check_pid_file(cgroups_pids[1], child);
}
static void mkdir_or_die(const char *path) {
if (mkdir(path, 0777) < 0) {
int err = errno;
printf("mkdir(%s) failed: %s\n", path, strerror(err));
exit(1);
}
}
static void touch_or_die(const char *path) {
FILE* f = fopen(path, "w");
if (!f) {
int err = errno;
printf("fopen(%s, w) failed: %s\n", path, strerror(err));
exit(1);
}
if (fclose(f) < 0) {
int err = errno;
printf("fclose(%s) failed: %s\n", path, strerror(err));
exit(1);
}
}
static void symlink_or_die(const char *old, const char *new) {
if (symlink(old, new) < 0) {
int err = errno;
printf("symlink(%s, %s) failed: %s\n", old, new, strerror(err));
exit(1);
}
}
static void expect_type(const char *path, int mode) {
struct stat st_buf;
if (stat(path, &st_buf) < 0) {
int err = errno;
if (err == ENOENT) {
if (mode == 0) {
return;
}
printf("expect_type(%s): stat failed unexpectedly: %s\n",
path, strerror(err));
exit(1);
}
}
if (mode == 0) {
printf("expect_type(%s): we expected the file to be gone, but it "
"existed.\n", path);
exit(1);
}
if (!(st_buf.st_mode & mode)) {
printf("expect_type(%s): the file existed, but it had mode 0%4o, "
"which didn't have bit 0%4o\n", path, st_buf.st_mode, mode);
exit(1);
}
}
int recursive_unlink_children(const char *name);
void test_recursive_unlink_children() {
int ret;
mkdir_or_die(TEST_ROOT "/unlinkRoot");
mkdir_or_die(TEST_ROOT "/unlinkRoot/a");
touch_or_die(TEST_ROOT "/unlinkRoot/b");
mkdir_or_die(TEST_ROOT "/unlinkRoot/c");
touch_or_die(TEST_ROOT "/unlinkRoot/c/d");
touch_or_die(TEST_ROOT "/external");
symlink_or_die(TEST_ROOT "/external",
TEST_ROOT "/unlinkRoot/c/external");
ret = recursive_unlink_children(TEST_ROOT "/unlinkRoot");
if (ret != 0) {
printf("recursive_unlink_children(%s) failed: error %d\n",
TEST_ROOT "/unlinkRoot", ret);
exit(1);
}
// unlinkRoot should still exist.
expect_type(TEST_ROOT "/unlinkRoot", S_IFDIR);
// Other files under unlinkRoot should have been deleted.
expect_type(TEST_ROOT "/unlinkRoot/a", 0);
expect_type(TEST_ROOT "/unlinkRoot/b", 0);
expect_type(TEST_ROOT "/unlinkRoot/c", 0);
// We shouldn't have followed the symlink.
expect_type(TEST_ROOT "/external", S_IFREG);
// Clean up.
if (rmdir(TEST_ROOT "/unlinkRoot") < 0) {
int err = errno;
printf("failed to rmdir " TEST_ROOT "/unlinkRoot: %s\n", strerror(err));
}
if (unlink(TEST_ROOT "/external") < 0) {
int err = errno;
printf("failed to unlink " TEST_ROOT "/external: %s\n", strerror(err));
}
}
void test_sanitize_docker_command() {
char *input[] = {
@ -772,7 +866,7 @@ int main(int argc, char **argv) {
if (mkdirs(TEST_ROOT "/logs/userlogs", 0755) != 0) {
exit(1);
}
if (write_config_file(TEST_ROOT "/test.cfg", 1) != 0) {
exit(1);
}
@ -804,6 +898,9 @@ int main(int argc, char **argv) {
printf("\nStarting tests\n");
printf("\nTesting recursive_unlink_children()\n");
test_recursive_unlink_children();
printf("\nTesting resolve_config_path()\n");
test_resolve_config_path();