YARN-7333. container-executor fails to remove entries from a directory that is not writable or executable. Contributed by Jason Lowe.

(cherry picked from commit 8620140a6a3ec0117675ede06d92d830da3da551)
This commit is contained in:
Nathan Roberts 2017-10-16 16:36:51 -05:00
parent 23ec4a788a
commit a809ce8f50
2 changed files with 35 additions and 14 deletions

View File

@ -1731,18 +1731,19 @@ static int unlink_helper(int dirfd, const char *name, int flags) {
} }
/** /**
* Determine if an entry in a directory is a symlink. * Determine if an entry in a directory is another directory without following
* symlinks.
* *
* @param dirfd The directory file descriptor, or -1 if there is none. * @param dirfd The directory file descriptor, or -1 if there is none.
* @param name If dirfd is -1, this is the path to examine. * @param name If dirfd is -1, this is the path to examine.
* Otherwise, this is the file name in the directory to * Otherwise, this is the file name in the directory to
* examine. * examine.
* *
* @return 0 if the entry is not a symlink * @return 0 if the entry is a symlink or otherwise not a directory
* 1 if the entry is a symlink * 1 if the entry is a directory
* A negative errno code if we couldn't access the entry. * A negative errno code if we couldn't access the entry.
*/ */
static int is_symlink_helper(int dirfd, const char *name) static int is_dir_helper(int dirfd, const char *name)
{ {
struct stat stat; struct stat stat;
@ -1755,7 +1756,7 @@ static int is_symlink_helper(int dirfd, const char *name)
return -errno; return -errno;
} }
} }
return !!S_ISLNK(stat.st_mode); return !!S_ISDIR(stat.st_mode);
} }
static int recursive_unlink_helper(int dirfd, const char *name, static int recursive_unlink_helper(int dirfd, const char *name,
@ -1765,30 +1766,29 @@ static int recursive_unlink_helper(int dirfd, const char *name,
DIR *dfd = NULL; DIR *dfd = NULL;
struct stat stat; struct stat stat;
// Check to see if the file is a symlink. If so, delete the symlink rather // Check to see if the file is a directory. If not then we can unlink it now.
// than what it points to. ret = is_dir_helper(dirfd, name);
ret = is_symlink_helper(dirfd, name);
if (ret < 0) { if (ret < 0) {
// is_symlink_helper failed. // is_dir_helper failed.
if (ret == -ENOENT) { if (ret == -ENOENT) {
ret = 0; ret = 0;
goto done; goto done;
} }
ret = -ret; ret = -ret;
fprintf(LOGFILE, "is_symlink_helper(%s) failed: %s\n", fprintf(LOGFILE, "is_dir_helper(%s) failed: %s\n",
fullpath, strerror(ret)); fullpath, strerror(ret));
goto done; goto done;
} else if (ret == 1) { } else if (ret == 0) {
// is_symlink_helper determined that the path is a symlink. // is_dir_helper determined that the path is not a directory.
ret = unlink_helper(dirfd, name, 0); ret = unlink_helper(dirfd, name, 0);
if (ret) { if (ret) {
fprintf(LOGFILE, "failed to unlink symlink %s: %s\n", fprintf(LOGFILE, "failed to unlink %s: %s\n",
fullpath, strerror(ret)); fullpath, strerror(ret));
} }
goto done; goto done;
} }
// Open the file. We use O_NOFOLLOW here to ensure that we if a symlink was // Open the directory. We use O_NOFOLLOW here to ensure that if a symlink was
// swapped in by an attacker, we will fail to follow it rather than deleting // swapped in by an attacker, we will fail to follow it rather than deleting
// something we potentially should not. // something we potentially should not.
fd = open_helper(dirfd, name); fd = open_helper(dirfd, name);
@ -1829,6 +1829,19 @@ static int recursive_unlink_helper(int dirfd, const char *name,
goto done; goto done;
} }
} else { } else {
// make sure the directory has full user permissions
// so entries can be deleted
if ((stat.st_mode & S_IRWXU) != S_IRWXU) {
ret = chmod_helper(dirfd, name, 0700);
if (ret) {
if (ret == ENOENT) {
ret = 0;
goto done;
}
fprintf(LOGFILE, "chmod(%s) failed: %s\n", fullpath, strerror(ret));
goto done;
}
}
dfd = fdopendir(fd); dfd = fdopendir(fd);
if (!dfd) { if (!dfd) {
ret = errno; ret = errno;

View File

@ -270,6 +270,8 @@ void test_delete_container() {
char buffer[100000]; char buffer[100000];
sprintf(buffer, "mkdir -p %s/who/let/the/dogs/out/who/who", container_dir); sprintf(buffer, "mkdir -p %s/who/let/the/dogs/out/who/who", container_dir);
run(buffer); run(buffer);
sprintf(buffer, "mknod %s/who/let/the/dogs/out/who/who/p p", container_dir);
run(buffer);
sprintf(buffer, "touch %s", dont_touch); sprintf(buffer, "touch %s", dont_touch);
run(buffer); run(buffer);
@ -287,9 +289,15 @@ void test_delete_container() {
run(buffer); run(buffer);
sprintf(buffer, "chmod 000 %s/who/let/protect", container_dir); sprintf(buffer, "chmod 000 %s/who/let/protect", container_dir);
run(buffer); run(buffer);
// create a no execute permission directory
sprintf(buffer, "chmod 600 %s/who/let/the", container_dir);
run(buffer);
// create a no permission directory // create a no permission directory
sprintf(buffer, "chmod 000 %s/who/let", container_dir); sprintf(buffer, "chmod 000 %s/who/let", container_dir);
run(buffer); run(buffer);
// create a no write permission directory
sprintf(buffer, "chmod 500 %s/who", container_dir);
run(buffer);
// delete container directory // delete container directory
char * dirs[] = {app_dir, 0}; char * dirs[] = {app_dir, 0};