Merge pull request #3646 from brauner/2021-02-02/fixes

src/lxc/attach.c

@@ -229,7 +229,7 @@ static int userns_setup_ids(struct attach_context *ctx,
 	if (!(options->namespaces & CLONE_NEWUSER))
 		return 0;
 
-	f_uidmap = fdopen_at(ctx->dfd_init_pid, "uid_map", "re", PROTECT_OPEN, PROTECT_LOOKUP_ABSOLUTE);
+	f_uidmap = fdopen_at(ctx->dfd_init_pid, "uid_map", "re", PROTECT_OPEN, PROTECT_LOOKUP_BENEATH);
 	if (!f_uidmap)
 		return log_error_errno(-errno, errno, "Failed to open uid_map");
 
@@ -249,7 +249,7 @@ static int userns_setup_ids(struct attach_context *ctx,
 		}
 	}
 
-	f_gidmap = fdopen_at(ctx->dfd_init_pid, "gid_map", "re", PROTECT_OPEN, PROTECT_LOOKUP_ABSOLUTE);
+	f_gidmap = fdopen_at(ctx->dfd_init_pid, "gid_map", "re", PROTECT_OPEN, PROTECT_LOOKUP_BENEATH);
 	if (!f_gidmap)
 		return log_error_errno(-errno, errno, "Failed to open gid_map");
 
@@ -314,7 +314,7 @@ static int parse_init_status(struct attach_context *ctx, lxc_attach_options_t *o
 	bool caps_found = false;
 	int ret;
 
-	f = fdopen_at(ctx->dfd_init_pid, "status", "re", PROTECT_OPEN, PROTECT_LOOKUP_ABSOLUTE);
+	f = fdopen_at(ctx->dfd_init_pid, "status", "re", PROTECT_OPEN, PROTECT_LOOKUP_BENEATH);
 	if (!f)
 		return log_error_errno(-errno, errno, "Failed to open status file");
 
@@ -572,6 +572,8 @@ static void put_attach_context(struct attach_context *ctx)
 
 static int attach_context_container(struct attach_context *ctx)
 {
+	int fret = 0;
+
 	for (int i = 0; i < LXC_NS_MAX; i++) {
 		int ret;
 
@@ -579,16 +581,19 @@ static int attach_context_container(struct attach_context *ctx)
 			continue;
 
 		ret = setns(ctx->ns_fd[i], ns_info[i].clone_flag);
-		if (ret < 0)
-			return log_error_errno(-1, errno,
-					       "Failed to attach to %s namespace of %d",
-					       ns_info[i].proc_name, ctx->init_pid);
+		if (ret)
+			return log_error_errno(-errno, errno, "Failed to attach to %s namespace of %d", ns_info[i].proc_name, ctx->init_pid);
 
-		DEBUG("Attached to %s namespace of %d",
-		ns_info[i].proc_name, ctx->init_pid);
+		DEBUG("Attached to %s namespace of %d", ns_info[i].proc_name, ctx->init_pid);
+
+		if (close(ctx->ns_fd[i])) {
+			fret = -errno;
+			SYSERROR("Failed to close file descriptor for %s namespace", ns_info[i].proc_name);
+		}
+		ctx->ns_fd[i] = -EBADF;
 	}
 
-	return 0;
+	return fret;
 }
 
 /*
@@ -1125,18 +1130,6 @@ __noreturn static void do_attach(struct attach_payload *ap)
 		TRACE("Set PR_SET_NO_NEW_PRIVS");
 	}
 
-	if (conf->seccomp.seccomp) {
-		ret = lxc_seccomp_load(conf);
-		if (ret < 0)
-			goto on_error;
-
-		TRACE("Loaded seccomp profile");
-
-		ret = lxc_seccomp_send_notifier_fd(&conf->seccomp, ap->ipc_socket);
-		if (ret < 0)
-			goto on_error;
-	}
-
 	/* The following is done after the communication socket is shut down.
 	 * That way, all errors that might (though unlikely) occur up until this
 	 * point will have their messages printed to the original stderr (if
@@ -1205,6 +1198,18 @@ __noreturn static void do_attach(struct attach_payload *ap)
 	if (ret)
 		INFO("Failed to adjust stdio permissions");
 
+	if (conf->seccomp.seccomp) {
+		ret = lxc_seccomp_load(conf);
+		if (ret < 0)
+			goto on_error;
+
+		TRACE("Loaded seccomp profile");
+
+		ret = lxc_seccomp_send_notifier_fd(&conf->seccomp, ap->ipc_socket);
+		if (ret < 0)
+			goto on_error;
+	}
+
 	if (!lxc_switch_uid_gid(ctx->target_ns_uid, ctx->target_ns_gid))
 		goto on_error;
 
@@ -1436,9 +1441,6 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 			_exit(EXIT_FAILURE);
 		}
 
-		/* close namespace file descriptors */
-		close_nsfds(ctx);
-
 		/* Attach succeeded, try to cwd. */
 		if (options->initial_cwd)
 			new_cwd = options->initial_cwd;

src/lxc/cgroups/cgfsng.c

@@ -3039,6 +3039,7 @@ __cgfsng_ops static bool cgfsng_devices_activate(struct cgroup_ops *ops, struct
 
 static bool __cgfsng_delegate_controllers(struct cgroup_ops *ops, const char *cgroup)
 {
+	__do_close int fd_base = -EBADF;
 	__do_free char *add_controllers = NULL, *base_path = NULL;
 	__do_free_string_list char **parts = NULL;
 	struct hierarchy *unified = ops->unified;
@@ -3070,6 +3071,14 @@ static bool __cgfsng_delegate_controllers(struct cgroup_ops *ops, const char *cg
 			(void)strlcat(add_controllers, " ", full_len + 1);
 	}
 
+	base_path = must_make_path(unified->mountpoint, unified->container_base_path, NULL);
+	fd_base = lxc_open_dirfd(base_path);
+	if (fd_base < 0)
+		return false;
+
+	if (!unified_cgroup_fd(fd_base))
+		return log_error_errno(false, EINVAL, "File descriptor does not refer to cgroup2 filesystem");
+
 	parts = lxc_string_split(cgroup, '/');
 	if (!parts)
 		return false;
@@ -3078,19 +3087,26 @@ static bool __cgfsng_delegate_controllers(struct cgroup_ops *ops, const char *cg
 	if (parts_len > 0)
 		parts_len--;
 
-	base_path = must_make_path(unified->mountpoint, unified->container_base_path, NULL);
 	for (ssize_t i = -1; i < parts_len; i++) {
 		int ret;
-		__do_free char *target = NULL;
 
-		if (i >= 0)
-			base_path = must_append_path(base_path, parts[i], NULL);
-		target = must_make_path(base_path, "cgroup.subtree_control", NULL);
-		ret = lxc_writeat(-1, target, add_controllers, full_len);
+		if (i >= 0) {
+			int fd_next;
+
+			fd_next = openat(fd_base, parts[i], PROTECT_OPATH_DIRECTORY, PROTECT_LOOKUP_BENEATH);
+			if (fd_next < 0)
+				return log_error_errno(false, errno, "Failed to open %d(%s)", fd_next, parts[i]);
+			close_prot_errno_move(fd_base, fd_next);
+		}
+
+		ret = lxc_writeat(fd_base, "cgroup.subtree_control", add_controllers, full_len);
 		if (ret < 0)
-			return log_error_errno(false, errno, "Could not enable \"%s\" controllers in the unified cgroup \"%s\"",
-					       add_controllers, target);
-		TRACE("Enable \"%s\" controllers in the unified cgroup \"%s\"", add_controllers, target);
+			return log_error_errno(false, errno,
+					       "Could not enable \"%s\" controllers in the unified cgroup %d(%s)",
+					       add_controllers, fd_base, (i >= 0) ? parts[i] : unified->container_base_path);
+
+		TRACE("Enable \"%s\" controllers in the unified cgroup %d(%s)",
+		      add_controllers, fd_base, (i >= 0) ? parts[i] : unified->container_base_path);
 	}
 
 	return true;

src/lxc/cgroups/cgroup_utils.c

@@ -98,3 +98,16 @@ int unified_cgroup_hierarchy(void)
 
 	return 0;
 }
+
+int unified_cgroup_fd(int fd)
+{
+
+	int ret;
+	struct statfs fs;
+
+	ret = fstatfs(fd, &fs);
+	if (!ret && is_fs_type(&fs, CGROUP2_SUPER_MAGIC))
+		return true;
+
+	return false;
+}

src/lxc/cgroups/cgroup_utils.h

@@ -30,4 +30,6 @@ __hidden extern bool test_writeable_v2(char *mountpoint, char *path);
 
 __hidden extern int unified_cgroup_hierarchy(void);
 
+__hidden extern int unified_cgroup_fd(int fd);
+
 #endif /* __LXC_CGROUP_UTILS_H */

src/lxc/file_utils.c

@@ -23,7 +23,7 @@
 
 int lxc_open_dirfd(const char *dir)
 {
-	return open(dir, O_DIRECTORY | O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
+	return open_at(-EBADF, dir, PROTECT_OPATH_DIRECTORY, PROTECT_LOOKUP_ABSOLUTE & ~RESOLVE_NO_XDEV, 0);
 }
 
 int lxc_readat(int dirfd, const char *filename, void *buf, size_t count)
@@ -47,8 +47,7 @@ int lxc_writeat(int dirfd, const char *filename, const void *buf, size_t count)
 	__do_close int fd = -EBADF;
 	ssize_t ret;
 
-	fd = openat(dirfd, filename,
-		    O_WRONLY | O_CLOEXEC | O_NOCTTY | O_NOFOLLOW);
+	fd = open_at(dirfd, filename, PROTECT_OPEN_W_WITH_TRAILING_SYMLINKS, PROTECT_LOOKUP_BENEATH, 0);
 	if (fd < 0)
 		return -1;
 

src/lxc/memory_utils.h

@@ -29,6 +29,15 @@
 		fd = -EBADF;        \
 	}
 
+#define close_prot_errno_move(fd, new_fd) \
+	if (fd >= 0) {                       \
+		int _e_ = errno;             \
+		close(fd);                   \
+		errno = _e_;                 \
+		fd = new_fd;                 \
+		new_fd = -EBADF;	     \
+	}
+
 static inline void close_prot_errno_disarm_function(int *fd)
 {
        close_prot_errno_disarm(*fd);

src/lxc/syscall_wrappers.h

@@ -268,6 +268,9 @@ struct lxc_open_how {
 #define PROTECT_OPEN_WITH_TRAILING_SYMLINKS (O_CLOEXEC | O_NOCTTY | O_RDONLY)
 #define PROTECT_OPEN (PROTECT_OPEN_WITH_TRAILING_SYMLINKS | O_NOFOLLOW)
 
+#define PROTECT_OPEN_W_WITH_TRAILING_SYMLINKS (O_CLOEXEC | O_NOCTTY | O_WRONLY)
+#define PROTECT_OPEN_W (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)
 {