Merge pull request #3644 from brauner/2021-02-01/fixes_3

src/lxc/attach.c

@@ -104,24 +104,22 @@ struct attach_context {
 	struct lsm_ops *lsm_ops;
 };
 
-static pid_t pidfd_get_pid(int pidfd)
+static pid_t pidfd_get_pid(int dfd_init_pid, int pidfd)
 {
 	__do_free char *line = NULL;
 	__do_fclose FILE *f = NULL;
 	size_t len = 0;
-	char path[STRLITERALLEN("/proc/self/fdinfo/") +
-		  INTTYPE_TO_STRLEN(int) + 1 ] = "/proc/self/fdinfo/";
+	char path[STRLITERALLEN("fdinfo/") + INTTYPE_TO_STRLEN(int) + 1 ] = "fdinfo/";
 	int ret;
 
-	if (pidfd < 0)
-		return -EBADF;
+	if (dfd_init_pid < 0 || pidfd < 0)
+		return ret_errno(EBADF);
 
-	ret = snprintf(path + STRLITERALLEN("/proc/self/fdinfo/"),
-			INTTYPE_TO_STRLEN(int), "%d", pidfd);
+	ret = snprintf(path + STRLITERALLEN("fdinfo/"), INTTYPE_TO_STRLEN(int), "%d", pidfd);
 	if (ret < 0 || ret > (size_t)INTTYPE_TO_STRLEN(int))
 		return ret_errno(EIO);
 
-	f = fopen_cloexec(path, "re");
+	f = fdopen_at(dfd_init_pid, path, "re", PROTECT_OPEN, PROTECT_LOOKUP_BENEATH);
 	if (!f)
 		return -errno;
 
@@ -231,7 +229,7 @@ static int userns_setup_ids(struct attach_context *ctx,
 	if (!(options->namespaces & CLONE_NEWUSER))
 		return 0;
 
-	f_uidmap = fdopenat(ctx->dfd_init_pid, "uid_map", "re");
+	f_uidmap = fdopen_at(ctx->dfd_init_pid, "uid_map", "re", PROTECT_OPEN, PROTECT_LOOKUP_ABSOLUTE);
 	if (!f_uidmap)
 		return log_error_errno(-errno, errno, "Failed to open uid_map");
 
@@ -251,7 +249,7 @@ static int userns_setup_ids(struct attach_context *ctx,
 		}
 	}
 
-	f_gidmap = fdopenat(ctx->dfd_init_pid, "gid_map", "re");
+	f_gidmap = fdopen_at(ctx->dfd_init_pid, "gid_map", "re", PROTECT_OPEN, PROTECT_LOOKUP_ABSOLUTE);
 	if (!f_gidmap)
 		return log_error_errno(-errno, errno, "Failed to open gid_map");
 
@@ -316,7 +314,7 @@ static int parse_init_status(struct attach_context *ctx, lxc_attach_options_t *o
 	bool caps_found = false;
 	int ret;
 
-	f = fdopenat(ctx->dfd_init_pid, "status", "re");
+	f = fdopen_at(ctx->dfd_init_pid, "status", "re", PROTECT_OPEN, PROTECT_LOOKUP_ABSOLUTE);
 	if (!f)
 		return log_error_errno(-errno, errno, "Failed to open status file");
 
@@ -380,24 +378,28 @@ static int get_attach_context(struct attach_context *ctx,
 	ctx->container = container;
 	ctx->attach_flags = options->attach_flags;
 
+	ctx->dfd_self_pid = open_at(-EBADF, "/proc/self",
+				    PROTECT_OPATH_FILE & ~O_NOFOLLOW,
+				    (PROTECT_LOOKUP_ABSOLUTE_WITH_SYMLINKS & ~RESOLVE_NO_XDEV), 0);
+	if (ctx->dfd_self_pid < 0)
+		return log_error_errno(-errno, errno, "Failed to open /proc/self");
+
 	init_pidfd = lxc_cmd_get_init_pidfd(container->name, container->config_path);
 	if (init_pidfd >= 0)
-		ctx->init_pid = pidfd_get_pid(init_pidfd);
+		ctx->init_pid = pidfd_get_pid(ctx->dfd_self_pid, init_pidfd);
 	else
 		ctx->init_pid = lxc_cmd_get_init_pid(container->name, container->config_path);
 
 	if (ctx->init_pid < 0)
 		return log_error(-1, "Failed to get init pid");
 
-	ctx->dfd_self_pid = openat(-EBADF, "/proc/self", O_CLOEXEC | O_NOCTTY | O_PATH | O_DIRECTORY);
-	if (ctx->dfd_self_pid < 0)
-		return log_error_errno(-errno, errno, "Failed to open /proc/self");
-
 	ret = snprintf(path, sizeof(path), "/proc/%d", ctx->init_pid);
 	if (ret < 0 || ret >= sizeof(path))
 		return ret_errno(EIO);
 
-	ctx->dfd_init_pid = openat(-EBADF, path, O_CLOEXEC | O_NOCTTY | O_NOFOLLOW | O_PATH | O_DIRECTORY);
+	ctx->dfd_init_pid = open_at(-EBADF, path,
+				    PROTECT_OPATH_DIRECTORY,
+				    (PROTECT_LOOKUP_ABSOLUTE & ~RESOLVE_NO_XDEV), 0);
 	if (ctx->dfd_init_pid < 0)
 		return log_error_errno(-errno, errno, "Failed to open /proc/%d", ctx->init_pid);
 
@@ -460,24 +462,30 @@ static int get_attach_context(struct attach_context *ctx,
 	return 0;
 }
 
-static int in_same_namespace(int ns_fd_pid1, int ns_fd_pid2, const char *ns_path)
+static int same_ns(int ns_fd_pid1, int ns_fd_pid2, const char *ns_path)
 {
 	__do_close int ns_fd1 = -EBADF, ns_fd2 = -EBADF;
 	int ret = -1;
 	struct stat ns_st1, ns_st2;
 
-	ns_fd1 = openat(ns_fd_pid1, ns_path, O_CLOEXEC | O_NOCTTY | O_RDONLY);
+	ns_fd1 = open_at(ns_fd_pid1, ns_path,
+			 PROTECT_OPEN_WITH_TRAILING_SYMLINKS,
+			 (PROTECT_LOOKUP_BENEATH_WITH_MAGICLINKS & ~(RESOLVE_NO_XDEV | RESOLVE_BENEATH)),
+			 0);
 	if (ns_fd1 < 0) {
 		/* The kernel does not support this namespace. This is not an error. */
 		if (errno == ENOENT)
 			return -EINVAL;
 
-		return -1;
+		return log_error_errno(-errno, errno, "Failed to open %d(%s)", ns_fd_pid1, ns_path);
 	}
 
-	ns_fd2 = openat(ns_fd_pid2, ns_path, O_CLOEXEC | O_NOCTTY | O_RDONLY);
+	ns_fd2 = open_at(ns_fd_pid2, ns_path,
+			 PROTECT_OPEN_WITH_TRAILING_SYMLINKS,
+			 (PROTECT_LOOKUP_BENEATH_WITH_MAGICLINKS & ~(RESOLVE_NO_XDEV | RESOLVE_BENEATH)),
+			 0);
 	if (ns_fd2 < 0)
-		return -1;
+		return log_error_errno(-errno, errno, "Failed to open %d(%s)", ns_fd_pid2, ns_path);
 
 	ret = fstat(ns_fd1, &ns_st1);
 	if (ret < 0)
@@ -503,9 +511,15 @@ static int get_attach_context_nsfds(struct attach_context *ctx,
 		int j;
 
 		if (options->namespaces & ns_info[i].clone_flag)
-			ctx->ns_fd[i] = openat(ctx->dfd_init_pid, ns_info[i].proc_path, O_CLOEXEC | O_NOCTTY | O_RDONLY);
+			ctx->ns_fd[i] = open_at(ctx->dfd_init_pid,
+						ns_info[i].proc_path,
+						PROTECT_OPEN_WITH_TRAILING_SYMLINKS,
+						(PROTECT_LOOKUP_BENEATH_WITH_MAGICLINKS & ~(RESOLVE_NO_XDEV | RESOLVE_BENEATH)),
+						0);
 		else if (ctx->ns_inherited & ns_info[i].clone_flag)
-			ctx->ns_fd[i] = in_same_namespace(ctx->dfd_self_pid, ctx->dfd_init_pid, ns_info[i].proc_path);
+			ctx->ns_fd[i] = same_ns(ctx->dfd_self_pid,
+						ctx->dfd_init_pid,
+						ns_info[i].proc_path);
 		else
 			continue;
 

src/lxc/cgroups/cgfsng.c

@@ -324,7 +324,7 @@ static bool cg_legacy_filter_and_set_cpus(const char *parent_cgroup,
 	bool flipped_bit = false;
 
 	fpath = must_make_path(parent_cgroup, "cpuset.cpus", NULL);
-	posscpus = read_file_at(-EBADF, fpath);
+	posscpus = read_file_at(-EBADF, fpath, PROTECT_OPEN, 0);
 	if (!posscpus)
 		return log_error_errno(false, errno, "Failed to read file \"%s\"", fpath);
 
@@ -334,7 +334,7 @@ static bool cg_legacy_filter_and_set_cpus(const char *parent_cgroup,
 		return false;
 
 	if (file_exists(__ISOL_CPUS)) {
-		isolcpus = read_file_at(-EBADF, __ISOL_CPUS);
+		isolcpus = read_file_at(-EBADF, __ISOL_CPUS, PROTECT_OPEN, 0);
 		if (!isolcpus)
 			return log_error_errno(false, errno, "Failed to read file \"%s\"", __ISOL_CPUS);
 
@@ -353,7 +353,7 @@ static bool cg_legacy_filter_and_set_cpus(const char *parent_cgroup,
 	}
 
 	if (file_exists(__OFFLINE_CPUS)) {
-		offlinecpus = read_file_at(-EBADF, __OFFLINE_CPUS);
+		offlinecpus = read_file_at(-EBADF, __OFFLINE_CPUS, PROTECT_OPEN, 0);
 		if (!offlinecpus)
 			return log_error_errno(false, errno, "Failed to read file \"%s\"", __OFFLINE_CPUS);
 
@@ -672,7 +672,7 @@ static char **cg_unified_get_controllers(int dfd, const char *file)
 	char *sep = " \t\n";
 	char *tok;
 
-	buf = read_file_at(dfd, file);
+	buf = read_file_at(dfd, file, PROTECT_OPEN, 0);
 	if (!buf)
 		return NULL;
 
@@ -3145,7 +3145,7 @@ static void cg_unified_delegate(char ***delegate)
 	char *token;
 	int idx;
 
-	buf = read_file_at(-EBADF, "/sys/kernel/cgroup/delegate");
+	buf = read_file_at(-EBADF, "/sys/kernel/cgroup/delegate", PROTECT_OPEN, 0);
 	if (!buf) {
 		for (char **p = standard; p && *p; p++) {
 			idx = append_null_to_list((void ***)delegate);
@@ -3183,9 +3183,9 @@ static int cg_hybrid_init(struct cgroup_ops *ops, bool relative, bool unprivileg
 	 * cgroups as our base in that case.
 	 */
 	if (!relative && (geteuid() == 0))
-		basecginfo = read_file_at(-EBADF, "/proc/1/cgroup");
+		basecginfo = read_file_at(-EBADF, "/proc/1/cgroup", PROTECT_OPEN, 0);
 	else
-		basecginfo = read_file_at(-EBADF, "/proc/self/cgroup");
+		basecginfo = read_file_at(-EBADF, "/proc/self/cgroup", PROTECT_OPEN, 0);
 	if (!basecginfo)
 		return ret_set_errno(-1, ENOMEM);
 
@@ -3314,9 +3314,9 @@ static char *cg_unified_get_current_cgroup(bool relative)
 	char *base_cgroup;
 
 	if (!relative && (geteuid() == 0))
-		basecginfo = read_file_at(-EBADF, "/proc/1/cgroup");
+		basecginfo = read_file_at(-EBADF, "/proc/1/cgroup", PROTECT_OPEN, 0);
 	else
-		basecginfo = read_file_at(-EBADF, "/proc/self/cgroup");
+		basecginfo = read_file_at(-EBADF, "/proc/self/cgroup", PROTECT_OPEN, 0);
 	if (!basecginfo)
 		return NULL;
 

src/lxc/file_utils.c

@@ -553,7 +553,8 @@ static inline int dup_cloexec(int fd)
 	return move_fd(fd_dup);
 }
 
-FILE *fdopenat(int dfd, const char *path, const char *mode)
+FILE *fdopen_at(int dfd, const char *path, const char *mode,
+		unsigned int o_flags, unsigned int resolve_flags)
 {
 	__do_close int fd = -EBADF;
 	__do_fclose FILE *f = NULL;
@@ -561,7 +562,7 @@ FILE *fdopenat(int dfd, const char *path, const char *mode)
 	if (is_empty_string(path))
 		fd = dup_cloexec(dfd);
 	else
-		fd = openat(dfd, path, O_CLOEXEC | O_NOCTTY | O_NOFOLLOW);
+		fd = open_at(dfd, path, o_flags, resolve_flags, 0);
 	if (fd < 0)
 		return NULL;
 
@@ -621,22 +622,24 @@ bool exists_file_at(int dir_fd, const char *path)
 	return fstatat(dir_fd, path, &sb, 0) == 0;
 }
 
-int open_beneath(int dir_fd, const char *path, unsigned int flags)
+int open_at(int dfd, const char *path, unsigned int o_flags,
+	    unsigned int resolve_flags, mode_t mode)
 {
 	__do_close int fd = -EBADF;
 	struct lxc_open_how how = {
-		.flags		= flags,
-		.resolve	= RESOLVE_NO_XDEV | RESOLVE_NO_SYMLINKS | RESOLVE_NO_MAGICLINKS | RESOLVE_BENEATH,
+		.flags		= o_flags,
+		.mode		= mode,
+		.resolve	= resolve_flags,
 	};
 
-	fd = openat2(dir_fd, path, &how, sizeof(how));
+	fd = openat2(dfd, path, &how, sizeof(how));
 	if (fd >= 0)
 		return move_fd(fd);
 
 	if (errno != ENOSYS)
 		return -errno;
 
-	return openat(dir_fd, path, O_NOFOLLOW | flags);
+	return openat(dfd, path, o_flags, mode);
 }
 
 int fd_make_nonblocking(int fd)
@@ -671,7 +674,8 @@ static void append_line(char **dest, size_t oldlen, char *new, size_t newlen)
 }
 
 /* Slurp in a whole file */
-char *read_file_at(int dfd, const char *fnam)
+char *read_file_at(int dfd, const char *fnam,
+		   unsigned int o_flags, unsigned resolve_flags)
 {
 	__do_close int fd = -EBADF;
 	__do_free char *buf = NULL, *line = NULL;
@@ -679,7 +683,7 @@ char *read_file_at(int dfd, const char *fnam)
 	size_t len = 0, fulllen = 0;
 	int linelen;
 
-	fd = openat(dfd, fnam, O_NOCTTY | O_CLOEXEC | O_NOFOLLOW | O_RDONLY);
+	fd = open_at(dfd, fnam, o_flags, resolve_flags, 0);
 	if (fd < 0)
 		return NULL;
 

src/lxc/file_utils.h

@@ -13,6 +13,7 @@
 #include <unistd.h>
 
 #include "compiler.h"
+#include "syscall_wrappers.h"
 
 /* read and write whole files */
 __hidden extern int lxc_write_to_file(const char *filename, const void *buf, size_t count,
@@ -76,13 +77,22 @@ static inline int fd_to_fd(int from, int to)
 __hidden extern int fd_cloexec(int fd, bool cloexec);
 __hidden extern int lxc_open_dirfd(const char *dir);
 __hidden extern FILE *fdopen_cached(int fd, const char *mode, void **caller_freed_buffer);
-__hidden extern FILE *fdopenat(int dfd, const char *path, const char *mode);
+__hidden extern FILE *fdopen_at(int dfd, const char *path, const char *mode,
+                                unsigned int o_flags,
+                                unsigned int resolve_flags);
 __hidden extern FILE *fopen_cached(const char *path, const char *mode, void **caller_freed_buffer);
 __hidden extern int timens_offset_write(clockid_t clk_id, int64_t s_offset, int64_t ns_offset);
 __hidden extern bool exists_dir_at(int dir_fd, const char *path);
 __hidden extern bool exists_file_at(int dir_fd, const char *path);
-__hidden extern int open_beneath(int dir_fd, const char *path, unsigned int flags);
+__hidden extern int open_at(int dfd, const char *path, unsigned int o_flags,
+			    unsigned int resolve_flags, mode_t mode);
+static inline int open_beneath(int dfd, const char *path, unsigned int flags)
+{
+	return open_at(dfd, path, flags, PROTECT_LOOKUP_BENEATH, 0);
+}
 __hidden int fd_make_nonblocking(int fd);
-__hidden extern char *read_file_at(int dfd, const char *fnam);
+__hidden extern char *read_file_at(int dfd, const char *fnam,
+                                   unsigned int o_flags,
+                                   unsigned resolve_flags);
 
 #endif /* __LXC_FILE_UTILS_H */

src/lxc/lsm/apparmor.c

@@ -16,6 +16,7 @@
 #include "conf.h"
 #include "config.h"
 #include "initutils.h"
+#include "file_utils.h"
 #include "log.h"
 #include "lsm.h"
 #include "parse.h"
@@ -446,7 +447,7 @@ static char *apparmor_process_label_get_at(struct lsm_ops *ops, int fd_pid)
 	__do_free char *label = NULL;
 	size_t len;
 
-	label = read_file_at(fd_pid, "attr/current");
+	label = read_file_at(fd_pid, "attr/current", PROTECT_OPEN, PROTECT_LOOKUP_BENEATH);
 	if (!label)
 		return log_error_errno(NULL, errno, "Failed to get AppArmor context");
 

src/lxc/lsm/selinux.c

@@ -13,6 +13,7 @@
 
 #include "conf.h"
 #include "config.h"
+#include "file_utils.h"
 #include "log.h"
 #include "lsm.h"
 #include "memory_utils.h"
@@ -56,7 +57,7 @@ static char *selinux_process_label_get_at(struct lsm_ops *ops, int fd_pid)
 	__do_free char *label = NULL;
 	size_t len;
 
-	label = read_file_at(fd_pid, "attr/current");
+	label = read_file_at(fd_pid, "attr/current", PROTECT_OPEN, PROTECT_LOOKUP_BENEATH);
 	if (!label)
 		return log_error_errno(NULL, errno, "Failed to get SELinux context");
 

src/lxc/syscall_wrappers.h

@@ -254,6 +254,20 @@ struct lxc_open_how {
 					(similar to chroot(2)). */
 #endif
 
+#define PROTECT_LOOKUP_BENEATH  (RESOLVE_BENEATH | RESOLVE_NO_XDEV | RESOLVE_NO_MAGICLINKS | RESOLVE_NO_SYMLINKS)
+#define PROTECT_LOOKUP_BENEATH_WITH_SYMLINKS (PROTECT_LOOKUP_BENEATH & ~RESOLVE_NO_SYMLINKS)
+#define PROTECT_LOOKUP_BENEATH_WITH_MAGICLINKS (PROTECT_LOOKUP_BENEATH & ~(RESOLVE_NO_SYMLINKS | RESOLVE_NO_MAGICLINKS))
+
+#define PROTECT_LOOKUP_ABSOLUTE (PROTECT_LOOKUP_BENEATH & ~RESOLVE_BENEATH)
+#define PROTECT_LOOKUP_ABSOLUTE_WITH_SYMLINKS (PROTECT_LOOKUP_ABSOLUTE & ~RESOLVE_NO_SYMLINKS)
+#define PROTECT_LOOKUP_ABSOLUTE_WITH_MAGICLINKS (PROTECT_LOOKUP_ABSOLUTE & ~(RESOLVE_NO_SYMLINKS | RESOLVE_NO_MAGICLINKS))
+
+#define PROTECT_OPATH_FILE (O_NOFOLLOW | O_PATH | O_CLOEXEC)
+#define PROTECT_OPATH_DIRECTORY (PROTECT_OPATH_FILE | O_DIRECTORY)
+
+#define PROTECT_OPEN_WITH_TRAILING_SYMLINKS (O_CLOEXEC | O_NOCTTY | O_RDONLY)
+#define PROTECT_OPEN (PROTECT_OPEN_WITH_TRAILING_SYMLINKS | O_NOFOLLOW)
+
 #ifndef HAVE_OPENAT2
 static inline int openat2(int dfd, const char *filename, struct lxc_open_how *how, size_t size)
 {