Merge pull request #3666 from brauner/2021-02-11/fixes

src/lxc/cgroups/cgfsng.c

@@ -1800,7 +1800,7 @@ static int __cg_mount_direct(int type, struct hierarchy *h,
 		fstype = "cgroup";
 	}
 
-	if (new_mount_api()) {
+	if (can_use_mount_api()) {
 		fd_fs = fs_prepare(fstype, -EBADF, "", 0, 0);
 		if (fd_fs < 0)
 			return log_error_errno(-errno, errno, "Failed to prepare filesystem context for %s", fstype);
@@ -1946,7 +1946,7 @@ __cgfsng_ops static bool cgfsng_mount(struct cgroup_ops *ops,
 	 * relying on RESOLVE_BENEATH so we need to skip the leading "/" in the
 	 * DEFAULT_CGROUP_MOUNTPOINT define.
 	 */
-	if (new_mount_api()) {
+	if (can_use_mount_api()) {
 		fd_fs = fs_prepare("tmpfs", -EBADF, "", 0, 0);
 		if (fd_fs < 0)
 			return log_error_errno(-errno, errno, "Failed to create new filesystem context for tmpfs");

src/lxc/conf.c

@@ -844,7 +844,7 @@ static int lxc_setup_ttys(struct lxc_conf *conf)
 						       "Failed to unlink %d(%s)",
 						       rootfs->dfd_dev, tty_name);
 
-			if (new_mount_api()) {
+			if (can_use_mount_api()) {
 				ret = fd_bind_mount(tty->pty, "",
 						    PROTECT_OPATH_FILE,
 						    PROTECT_LOOKUP_BENEATH_XDEV,
@@ -881,7 +881,7 @@ static int lxc_setup_ttys(struct lxc_conf *conf)
 						       "Failed to create tty mount target %d(%s)",
 						       rootfs->dfd_dev, rootfs->buf);
 
-			if (new_mount_api()) {
+			if (can_use_mount_api()) {
 				ret = fd_bind_mount(tty->pty, "",
 						    PROTECT_OPATH_FILE,
 						    PROTECT_LOOKUP_BENEATH_XDEV,
@@ -1074,7 +1074,7 @@ static int mount_autodev(const char *name, const struct lxc_rootfs *rootfs,
 		goto reset_umask;
 	}
 
-	if (new_mount_api()) {
+	if (can_use_mount_api()) {
 		fd_fs = fs_prepare("tmpfs", -EBADF, "", 0, 0);
 		if (fd_fs < 0)
 			return log_error_errno(-errno, errno, "Failed to prepare filesystem context for tmpfs");
@@ -1216,7 +1216,7 @@ static int lxc_fill_autodev(struct lxc_rootfs *rootfs)
 		if (ret < 0)
 			return ret_errno(EIO);
 
-		if (new_mount_api()) {
+		if (can_use_mount_api()) {
 			ret = fd_bind_mount(rootfs->dfd_host, rootfs->buf,
 					    PROTECT_OPATH_FILE,
 					    PROTECT_LOOKUP_BENEATH_XDEV,
@@ -1643,16 +1643,51 @@ static inline bool wants_console(const struct lxc_terminal *terminal)
 	return !terminal->path || strcmp(terminal->path, "none");
 }
 
+static int lxc_bind_mount_console(const struct lxc_terminal *console,
+				  int dfd_to, const char *path_to)
+{
+	__do_close int fd_pty = -EBADF;
+
+	if (is_empty_string(console->name))
+		return ret_errno(EINVAL);
+
+	/*
+	 * When the pty fd stashed in console->pty has been retrieved via the
+	 * TIOCGPTPEER ioctl() to avoid dangerous path-based lookups when
+	 * allocating new pty devices we can't reopen it through openat2() or
+	 * created a detached mount through open_tree() from it. This means we
+	 * would need to mount using the path stased in console->name which is
+	 * unsafe. We could be mounting a device that isn't identical to the
+	 * one we've already safely opened and stashed in console->pty.
+	 * So, what we do is we open an O_PATH file descriptor for
+	 * console->name and verify that the opened fd and the fd we stashed in
+	 * console->pty refer to the same device. If they do we can go on and
+	 * created a detached mount based on the newly opened O_PATH file
+	 * descriptor and then safely mount.
+	 */
+	fd_pty = open_at(-EBADF, console->name, PROTECT_OPATH_FILE,
+			 PROTECT_LOOKUP_ABSOLUTE_XDEV, 0);
+	if (fd_pty < 0)
+		return log_error_errno(-errno, errno, "Failed to open \"%s\"", console->name);
+
+	if (!same_file_lax(console->pty, fd_pty))
+		return log_error_errno(-EINVAL, EINVAL, "Console file descriptor changed");
+
+	/*
+	 * Note, there are intentionally no open or lookup restrictions since
+	 * we're operating directly on the fd.
+	 */
+	return fd_bind_mount(fd_pty, "", 0, 0,
+			     dfd_to, path_to, PROTECT_OPATH_FILE, PROTECT_LOOKUP_BENEATH,
+			     0, false);
+}
+
 static int lxc_setup_dev_console(struct lxc_rootfs *rootfs,
-				 const struct lxc_terminal *console,
-				 int pty_mnt_fd)
+				 const struct lxc_terminal *console)
 {
 	int ret;
 	char *rootfs_path = rootfs->path ? rootfs->mount : "";
 
-	if (!wants_console(console))
-		return 0;
-
 	/*
 	 * When we are asked to setup a console we remove any previous
 	 * /dev/console bind-mounts.
@@ -1681,47 +1716,30 @@ static int lxc_setup_dev_console(struct lxc_rootfs *rootfs,
 	if (ret < 0)
 		return log_error_errno(-errno, errno, "Failed to set mode \"0%o\" to \"%s\"", S_IXUSR | S_IXGRP, console->name);
 
-	if (pty_mnt_fd >= 0) {
-		ret = move_mount(pty_mnt_fd, "", rootfs->dfd_dev, "console", MOVE_MOUNT_F_EMPTY_PATH);
-		if (!ret) {
-			DEBUG("Moved mount \"%s\" onto %d(console)", console->name, rootfs->dfd_dev);
-			return 0;
-		}
-
-		if (ret && errno != ENOSYS)
-			return log_error_errno(-1, errno,
-					       "Failed to mount %d(%s) on %d(console)",
-					       pty_mnt_fd, console->name, rootfs->dfd_dev);
-	}
-
-	ret = safe_mount_beneath_at(rootfs->dfd_dev, console->name, "console", NULL, MS_BIND, NULL);
-	if (ret < 0) {
-		if (errno == ENOSYS) {
-			ret = strnprintf(rootfs->buf, sizeof(rootfs->buf), "%s/dev/console", rootfs_path);
-			if (ret < 0)
-				return -1;
+	if (can_use_mount_api()) {
+		ret = lxc_bind_mount_console(console, rootfs->dfd_dev, "console");
+	} else {
+		ret = strnprintf(rootfs->buf, sizeof(rootfs->buf), "%s/dev/console", rootfs_path);
+		if (ret < 0)
+			return ret;
 
-			ret = safe_mount(console->name, rootfs->buf, "none", MS_BIND, NULL, rootfs_path);
-			if (ret < 0)
-				return log_error_errno(-1, errno, "Failed to mount %d(%s) on \"%s\"", pty_mnt_fd, console->name, rootfs->buf);
-		}
+		ret = safe_mount(console->name, rootfs->buf, "none", MS_BIND, NULL, rootfs_path);
 	}
+	if (ret < 0)
+		return log_error_errno(ret, errno, "Failed to mount %d(%s) on \"%s\"", console->pty, console->name, rootfs->buf);
 
-	DEBUG("Mounted pty device %d(%s) onto \"%s\"", pty_mnt_fd, console->name, rootfs->buf);
+	DEBUG("Mounted pty device %d(%s) onto \"%s\"", console->pty, console->name, rootfs->buf);
 	return 0;
 }
 
-static int lxc_setup_ttydir_console(const struct lxc_rootfs *rootfs,
+static int lxc_setup_ttydir_console(struct lxc_rootfs *rootfs,
 				    const struct lxc_terminal *console,
-				    char *ttydir, int pty_mnt_fd)
+				    char *ttydir)
 {
 	int ret;
 	char path[PATH_MAX], lxcpath[PATH_MAX];
 	char *rootfs_path = rootfs->path ? rootfs->mount : "";
 
-	if (!wants_console(console))
-		return 0;
-
 	/* create rootfs/dev/<ttydir> directory */
 	ret = strnprintf(path, sizeof(path), "%s/dev/%s", rootfs_path, ttydir);
 	if (ret < 0)
@@ -1761,44 +1779,51 @@ static int lxc_setup_ttydir_console(const struct lxc_rootfs *rootfs,
 		return log_error_errno(-errno, errno, "Failed to set mode \"0%o\" to \"%s\"", S_IXUSR | S_IXGRP, console->name);
 
 	/* bind mount console->name to '/dev/<ttydir>/console' */
-	if (pty_mnt_fd >= 0) {
-		ret = move_mount(pty_mnt_fd, "", -EBADF, lxcpath, MOVE_MOUNT_F_EMPTY_PATH);
-		if (!ret) {
-			DEBUG("Moved mount \"%s\" onto \"%s\"", console->name, lxcpath);
-			goto finish;
-		}
+	if (can_use_mount_api()) {
+		ret = strnprintf(rootfs->buf, sizeof(rootfs->buf), "%s/console", ttydir);
+		if (ret < 0)
+			return ret;
 
-		if (ret && errno != ENOSYS)
-			return log_error_errno(-1, errno,
-					       "Failed to mount %d(%s) on \"%s\"",
-					       pty_mnt_fd, console->name, lxcpath);
+		ret = lxc_bind_mount_console(console, rootfs->dfd_dev, rootfs->buf);
+	} else {
+		ret = safe_mount(console->name, lxcpath, "none", MS_BIND, 0, rootfs_path);
 	}
-
-	ret = safe_mount(console->name, lxcpath, "none", MS_BIND, 0, rootfs_path);
 	if (ret < 0)
-		return log_error_errno(-1, errno, "Failed to mount %d(%s) on \"%s\"", pty_mnt_fd, console->name, lxcpath);
+		return log_error_errno(ret, errno, "Failed to mount %d(%s) on \"%s\"", console->pty, console->name, lxcpath);
 	DEBUG("Mounted \"%s\" onto \"%s\"", console->name, lxcpath);
 
-finish:
 	/* bind mount '/dev/<ttydir>/console'  to '/dev/console'  */
-	ret = safe_mount(lxcpath, path, "none", MS_BIND, 0, rootfs_path);
+	if (can_use_mount_api()) {
+		ret = fd_bind_mount(rootfs->dfd_dev, rootfs->buf,
+				    PROTECT_OPATH_FILE, PROTECT_LOOKUP_BENEATH_XDEV,
+				    rootfs->dfd_dev, "console",
+				    PROTECT_OPATH_FILE, PROTECT_LOOKUP_BENEATH,
+				    0, false);
+	} else {
+		ret = safe_mount(lxcpath, path, "none", MS_BIND, 0, rootfs_path);
+	}
 	if (ret < 0)
 		return log_error_errno(-1, errno, "Failed to mount \"%s\" on \"%s\"", console->name, lxcpath);
-	DEBUG("Mounted \"%s\" onto \"%s\"", console->name, lxcpath);
+	DEBUG("Mounted \"%s\" onto \"%s\"", lxcpath, path);
 
 	DEBUG("Console has been setup under \"%s\" and mounted to \"%s\"", lxcpath, path);
 	return 0;
 }
 
 static int lxc_setup_console(struct lxc_rootfs *rootfs,
-			     const struct lxc_terminal *console, char *ttydir,
-			     int pty_mnt_fd)
+			     struct lxc_terminal *console, char *ttydir)
 {
+	int ret;
 
-	if (!ttydir)
-		return lxc_setup_dev_console(rootfs, console, pty_mnt_fd);
+	if (!wants_console(console))
+		return log_trace(0, "Skipping console setup");
 
-	return lxc_setup_ttydir_console(rootfs, console, ttydir, pty_mnt_fd);
+	if (ttydir)
+		ret = lxc_setup_ttydir_console(rootfs, console, ttydir);
+	else
+		ret = lxc_setup_dev_console(rootfs, console);
+	close_prot_errno_disarm(console->pty);
+	return ret;
 }
 
 static int parse_mntopt(char *opt, unsigned long *flags, char **data, size_t size)
@@ -3361,7 +3386,6 @@ static int lxc_setup_keyring(struct lsm_ops *lsm_ops, const struct lxc_conf *con
 
 int lxc_setup(struct lxc_handler *handler)
 {
-	__do_close int pty_mnt_fd = -EBADF;
 	int ret;
 	const char *lxcpath = handler->lxcpath, *name = handler->name;
 	struct lxc_conf *lxc_conf = handler->conf;
@@ -3393,17 +3417,6 @@ int lxc_setup(struct lxc_handler *handler)
 			return log_error(-1, "Failed to send network device names and ifindices to parent");
 	}
 
-	if (wants_console(&lxc_conf->console)) {
-		pty_mnt_fd = open_tree(-EBADF, lxc_conf->console.name,
-				       OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC | AT_EMPTY_PATH);
-		if (pty_mnt_fd < 0)
-			SYSTRACE("Failed to create detached mount for container's console \"%s\"",
-				 lxc_conf->console.name);
-		else
-			TRACE("Created detached mount for container's console \"%s\"",
-			      lxc_conf->console.name);
-	}
-
 	if (lxc_conf->autodev > 0) {
 		ret = mount_autodev(name, &lxc_conf->rootfs, lxc_conf->autodevtmpfssize, lxcpath);
 		if (ret < 0)
@@ -3487,7 +3500,7 @@ int lxc_setup(struct lxc_handler *handler)
 		return log_error(-1, "Failed to \"/proc\" LSMs");
 
 	ret = lxc_setup_console(&lxc_conf->rootfs, &lxc_conf->console,
-				lxc_conf->ttys.dir, pty_mnt_fd);
+				lxc_conf->ttys.dir);
 	if (ret < 0)
 		return log_error(-1, "Failed to setup console");
 

src/lxc/file_utils.c

@@ -725,3 +725,26 @@ char *read_file_at(int dfd, const char *fnam,
 
 	return move_ptr(buf);
 }
+
+bool same_file_lax(int fda, int fdb)
+{
+	struct stat st_fda, st_fdb;
+
+
+        if (fda == fdb)
+                return true;
+
+        if (fstat(fda, &st_fda) < 0)
+                return false;
+
+        if (fstat(fdb, &st_fdb) < 0)
+                return false;
+
+	errno = EINVAL;
+	if ((st_fda.st_mode & S_IFMT) != (st_fdb.st_mode & S_IFMT))
+		return false;
+
+	errno = EINVAL;
+	return (st_fda.st_dev == st_fdb.st_dev) &&
+	       (st_fda.st_ino == st_fdb.st_ino);
+}

src/lxc/file_utils.h

@@ -97,4 +97,12 @@ __hidden extern char *read_file_at(int dfd, const char *fnam,
 __hidden extern ssize_t lxc_read_try_buf_at(int dfd, const char *path,
                                             void *buf, size_t count);
 
+/*
+ * Check if two fds refer to the same file.
+ * The function is "lax" in so far, as it doesn't care whether fda and fdb have
+ * the same flags or whether they share the same device context when they refer
+ * to devices.
+ */
+__hidden extern bool same_file_lax(int fda, int fdb);
+
 #endif /* __LXC_FILE_UTILS_H */

src/lxc/macro.h

@@ -309,6 +309,14 @@
 #define LXC_PROC_PID_FD_LEN \
 	(6 + INTTYPE_TO_STRLEN(pid_t) + 4 + INTTYPE_TO_STRLEN(int) + 1)
 
+/* /proc/self/fd/ =    14
+ *                   +
+ * <fd-as-str>    =    INTTYPE_TO_STRLEN(int)
+ *                   +
+ * \0           =      1
+ */
+#define LXC_PROC_SELF_FD_LEN (14 + INTTYPE_TO_STRLEN(int) + 1)
+
 /* /proc/        = 6
  *               +
  * <pid-as-str>  = INTTYPE_TO_STRLEN(pid_t)

src/lxc/memory_utils.h

@@ -89,4 +89,10 @@ static inline void *memdup(const void *data, size_t len)
 		(a) = move_ptr((b)); \
 	})
 
+#define close_move_fd(a, b)         \
+	({                          \
+		close(a);           \
+		(a) = move_fd((b)); \
+	})
+
 #endif /* __LXC_MEMORY_UTILS_H */

src/lxc/mount_utils.c

@@ -438,3 +438,53 @@ unsigned long add_required_remount_flags(const char *s, const char *d,
        return flags;
 #endif
 }
+
+bool can_use_mount_api(void)
+{
+	static int supported = -1;
+
+	if (supported == -1) {
+		__do_close int fd = -EBADF;
+
+		fd = openat2(-EBADF, "", NULL, 0);
+		if (fd > 0 || errno == ENOSYS) {
+			supported = 0;
+			return false;
+		}
+
+		fd = fsmount(-EBADF, 0, 0);
+		if (fd > 0 || errno == ENOSYS) {
+			supported = 0;
+			return false;
+		}
+
+		fd = fsconfig(-EBADF, -EINVAL, NULL, NULL, 0);
+		if (fd > 0 || errno == ENOSYS) {
+			supported = 0;
+			return false;
+		}
+
+		fd = fsopen(NULL, 0);
+		if (fd > 0 || errno == ENOSYS) {
+			supported = 0;
+			return false;
+		}
+
+		fd = move_mount(-EBADF, NULL, -EBADF, NULL, 0);
+		if (fd > 0 || errno == ENOSYS) {
+			supported = 0;
+			return false;
+		}
+
+		fd = open_tree(-EBADF, NULL, 0);
+		if (fd > 0 || errno == ENOSYS) {
+			supported = 0;
+			return false;
+		}
+
+		supported = 1;
+		TRACE("Kernel supports mount api");
+	}
+
+	return supported == 1;
+}

src/lxc/mount_utils.h

@@ -185,28 +185,6 @@ __hidden extern int fd_bind_mount(int dfd_from, const char *path_from,
 				  __u64 o_flags_to, __u64 resolve_flags_to,
 				  unsigned int attr_flags, bool recursive);
 
-/*
- * We use openat2() as indicator whether or not the new mount api is supported.
- * First, because openat2() has been introduced after all syscalls from the new
- * mount api we currently use and second because our hardened mount logic
- * relies on openat2() to safely resolve paths.
- */
-static inline bool new_mount_api(void)
-{
-	__do_close int fd = -EBADF;
-	static int supported = -1;
-
-	if (supported == -1) {
-		fd = openat2(-EBADF, "", NULL, 0);
-		if (fd < 0 && errno != ENOSYS)
-			supported = 1;
-		else
-			supported = 0;
-	}
-
-	return supported == 1;
-}
-
 __hidden extern int calc_remount_flags_new(int dfd_from, const char *path_from,
 					   __u64 o_flags_from,
 					   __u64 resolve_flags_from,
@@ -223,4 +201,6 @@ __hidden extern unsigned long add_required_remount_flags(const char *s,
 							 const char *d,
 							 unsigned long flags);
 
+__hidden extern bool can_use_mount_api(void);
+
 #endif /* __LXC_MOUNT_UTILS_H */

src/lxc/string_utils.h

@@ -149,4 +149,27 @@ static inline bool strequal(const char *str, const char *eq)
 		__ret_strnprintf;                                                 \
 	})
 
+static inline const char *proc_self_fd(int fd)
+{
+	static const char *invalid_fd_path = "/proc/self/fd/-EBADF";
+	static char buf[LXC_PROC_SELF_FD_LEN] = "/proc/self/fd/";
+
+	if (strnprintf(buf + STRLITERALLEN("/proc/self/fd/"),
+		       INTTYPE_TO_STRLEN(int), "%d", fd) < 0)
+		return invalid_fd_path;
+
+	return buf;
+}
+
+static inline const char *fdstr(int fd)
+{
+	static const char *fdstr_invalid = "-EBADF";
+	static char buf[INTTYPE_TO_STRLEN(int)];
+
+	if (strnprintf(buf, sizeof(buf), "%d", fd) < 0)
+		return fdstr_invalid;
+
+	return buf;
+}
+
 #endif /* __LXC_STRING_UTILS_H */