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)
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 3b76964..09a7b44 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
@@ -1870,18 +1870,19 @@
}
/**
- * 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 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
+ * @return 0 if the entry is a symlink or otherwise not a directory
+ * 1 if the entry is a directory
* 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;
@@ -1894,7 +1895,7 @@
return -errno;
}
}
- return !!S_ISLNK(stat.st_mode);
+ return !!S_ISDIR(stat.st_mode);
}
static int recursive_unlink_helper(int dirfd, const char *name,
@@ -1904,30 +1905,29 @@
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);
+ // Check to see if the file is a directory. If not then we can unlink it now.
+ ret = is_dir_helper(dirfd, name);
if (ret < 0) {
- // is_symlink_helper failed.
+ // is_dir_helper failed.
if (ret == -ENOENT) {
ret = 0;
goto done;
}
ret = -ret;
- fprintf(LOGFILE, "is_symlink_helper(%s) failed: %s\n",
+ fprintf(LOGFILE, "is_dir_helper(%s) failed: %s\n",
fullpath, strerror(ret));
goto done;
- } else if (ret == 1) {
- // is_symlink_helper determined that the path is a symlink.
+ } else if (ret == 0) {
+ // is_dir_helper determined that the path is not a directory.
ret = unlink_helper(dirfd, name, 0);
if (ret) {
- fprintf(LOGFILE, "failed to unlink symlink %s: %s\n",
+ fprintf(LOGFILE, "failed to unlink %s: %s\n",
fullpath, strerror(ret));
}
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
// something we potentially should not.
fd = open_helper(dirfd, name);
@@ -1968,6 +1968,19 @@
goto done;
}
} 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);
if (!dfd) {
ret = errno;
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
index 438ac94..c9ab507 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c
@@ -296,6 +296,8 @@
char buffer[100000];
sprintf(buffer, "mkdir -p %s/who/let/the/dogs/out/who/who", container_dir);
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);
run(buffer);
@@ -313,9 +315,15 @@
run(buffer);
sprintf(buffer, "chmod 000 %s/who/let/protect", container_dir);
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
sprintf(buffer, "chmod 000 %s/who/let", container_dir);
run(buffer);
+ // create a no write permission directory
+ sprintf(buffer, "chmod 500 %s/who", container_dir);
+ run(buffer);
// delete container directory
char * dirs[] = {app_dir, 0};
