Merge pull request #3914 from brauner/2021-07-29.devpts

devpts: move setup before pivot root

src/lxc/conf.c

@@ -1645,7 +1645,7 @@ static const struct id_map *find_mapped_nsid_entry(const struct lxc_conf *conf,
 	return retmap;
 }
 
-static int lxc_setup_devpts_parent(struct lxc_handler *handler)
+static int lxc_recv_devpts_from_child(struct lxc_handler *handler)
 {
 	int ret;
 
@@ -1663,89 +1663,87 @@ static int lxc_setup_devpts_parent(struct lxc_handler *handler)
 	return 0;
 }
 
-static int lxc_prepare_devpts_child(struct lxc_handler *handler)
+static int lxc_setup_devpts_child(struct lxc_handler *handler)
 {
-	__do_close int fd_fs = -EBADF, fd_fsmnt = -EBADF;
+	__do_close int devpts_fd = -EBADF, fd_fs = -EBADF;
 	struct lxc_conf *conf = handler->conf;
+	struct lxc_rootfs *rootfs = &conf->rootfs;
 	int ret;
 
-	if (!can_use_mount_api())
-		return 0;
-
 	if (conf->pty_max <= 0)
 		return log_debug(0, "No new devpts instance will be mounted since no pts devices are requested");
 
-	fd_fs = fs_prepare("devpts", -EBADF, "", 0, 0);
-	if (fd_fs < 0)
-		return syserror("Failed to prepare filesystem context for devpts");
-
-	ret = fs_set_property(fd_fs, "gid", "5");
+	ret = strnprintf(rootfs->buf, sizeof(rootfs->buf),
+			 "/proc/self/fd/%d/pts", rootfs->dfd_dev);
 	if (ret < 0)
-		SYSTRACE("Failed to set \"gid=5\" on devpts filesystem context %d", fd_fs);
+		return syserror("Failed to create path");
 
-	ret = fs_set_flag(fd_fs, "newinstance");
-	if (ret < 0)
-		return syserror("Failed to set \"newinstance\" property on devpts filesystem context %d", fd_fs);
+	(void)umount2(rootfs->buf, MNT_DETACH);
 
-	ret = fs_set_property(fd_fs, "ptmxmode", "0666");
-	if (ret < 0)
-		return syserror("Failed to set \"ptmxmode=0666\" property on devpts filesystem context %d", fd_fs);
+	/* Create mountpoint for devpts instance. */
+	ret = mkdirat(rootfs->dfd_dev, "pts", 0755);
+	if (ret < 0 && errno != EEXIST)
+		return log_error_errno(-1, errno, "Failed to create \"/dev/pts\" directory");
 
-	ret = fs_set_property(fd_fs, "mode", "0620");
-	if (ret < 0)
-		return syserror("Failed to set \"mode=0620\" property on devpts filesystem context %d", fd_fs);
+	if (can_use_mount_api()) {
+		fd_fs = fs_prepare("devpts", -EBADF, "", 0, 0);
+		if (fd_fs < 0)
+			return syserror("Failed to prepare filesystem context for devpts");
 
-	ret = fs_set_property(fd_fs, "max", fdstr(conf->pty_max));
-	if (ret < 0)
-		return syserror("Failed to set \"max=%zu\" property on devpts filesystem context %d", conf->pty_max, fd_fs);
+		ret = fs_set_property(fd_fs, "source", "devpts");
+		if (ret < 0)
+			SYSTRACE("Failed to set \"source=devpts\" on devpts filesystem context %d", fd_fs);
 
-	ret = fsconfig(fd_fs, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
-	if (ret < 0)
-		return syserror("Failed to finalize filesystem context %d", fd_fs);
+		ret = fs_set_property(fd_fs, "gid", "5");
+		if (ret < 0)
+			SYSTRACE("Failed to set \"gid=5\" on devpts filesystem context %d", fd_fs);
 
-	fd_fsmnt = fsmount(fd_fs, FSMOUNT_CLOEXEC, MOUNT_ATTR_NOSUID | MOUNT_ATTR_NOEXEC);
-	if (fd_fsmnt < 0)
-		return syserror("Failed to create new mount for filesystem context %d", fd_fs);
+		ret = fs_set_flag(fd_fs, "newinstance");
+		if (ret < 0)
+			return syserror("Failed to set \"newinstance\" property on devpts filesystem context %d", fd_fs);
 
-	TRACE("Created detached devpts mount %d", fd_fsmnt);
-	handler->conf->devpts_fd = move_fd(fd_fsmnt);
+		ret = fs_set_property(fd_fs, "ptmxmode", "0666");
+		if (ret < 0)
+			return syserror("Failed to set \"ptmxmode=0666\" property on devpts filesystem context %d", fd_fs);
 
-	return 0;
-}
+		ret = fs_set_property(fd_fs, "mode", "0620");
+		if (ret < 0)
+			return syserror("Failed to set \"mode=0620\" property on devpts filesystem context %d", fd_fs);
 
-static int lxc_finalize_devpts_child(struct lxc_handler *handler)
-{
-	int ret;
-	char **opts;
-	char devpts_mntopts[256];
-	char *mntopt_sets[5];
-	char default_devpts_mntopts[256] = "gid=5,newinstance,ptmxmode=0666,mode=0620";
-	struct lxc_conf *conf = handler->conf;
-	struct lxc_rootfs *rootfs = &conf->rootfs;
+		ret = fs_set_property(fd_fs, "max", fdstr(conf->pty_max));
+		if (ret < 0)
+			return syserror("Failed to set \"max=%zu\" property on devpts filesystem context %d", conf->pty_max, fd_fs);
 
-	if (conf->pty_max <= 0)
-		return log_debug(0, "No new devpts instance will be mounted since no pts devices are requested");
+		ret = fsconfig(fd_fs, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
+		if (ret < 0)
+			return syserror("Failed to finalize filesystem context %d", fd_fs);
 
-	/*
-	 * Fallback codepath in case the new mount API can't be used to create
-	 * detached mounts.
-	 */
-	if (conf->devpts_fd >= 0) {
-		ret = move_mount(conf->devpts_fd, "", rootfs->dfd_dev, "pts", MOVE_MOUNT_F_EMPTY_PATH);
+		devpts_fd = fsmount(fd_fs, FSMOUNT_CLOEXEC, MOUNT_ATTR_NOSUID | MOUNT_ATTR_NOEXEC);
+		if (devpts_fd < 0)
+			return syserror("Failed to create new mount for filesystem context %d", fd_fs);
+		TRACE("Created detached devpts mount %d", devpts_fd);
+
+		ret = move_mount(devpts_fd, "", rootfs->dfd_dev, "pts", MOVE_MOUNT_F_EMPTY_PATH);
 		if (ret)
 			return syserror("Failed to attach devpts mount %d to %d/pts", conf->devpts_fd, rootfs->dfd_dev);
 
-		DEBUG("Attached detached devpts mount %d to %d/pts", conf->devpts_fd, rootfs->dfd_dev);
+		DEBUG("Attached detached devpts mount %d to %d/pts", devpts_fd, rootfs->dfd_dev);
 	} else {
-		__do_close int devpts_fd = -EBADF;
+		char **opts;
+		char devpts_mntopts[256];
+		char *mntopt_sets[5];
+		char default_devpts_mntopts[256] = "gid=5,newinstance,ptmxmode=0666,mode=0620";
+
+		/*
+		 * Fallback codepath in case the new mount API can't be used to
+		 * create detached mounts.
+		 */
 
 		ret = strnprintf(devpts_mntopts, sizeof(devpts_mntopts), "%s,max=%zu",
 				default_devpts_mntopts, conf->pty_max);
 		if (ret < 0)
 			return -1;
 
-		(void)umount2("/dev/pts", MNT_DETACH);
-
 		/* Create mountpoint for devpts instance. */
 		ret = mkdirat(rootfs->dfd_dev, "pts", 0755);
 		if (ret < 0 && errno != EEXIST)
@@ -1768,11 +1766,10 @@ static int lxc_finalize_devpts_child(struct lxc_handler *handler)
 
 		for (ret = -1, opts = mntopt_sets; opts && *opts; opts++) {
 			/* mount new devpts instance */
-			ret = mount("devpts", "/dev/pts", "devpts", MS_NOSUID | MS_NOEXEC, *opts);
+			ret = mount_beneath_fd(rootfs->dfd_dev, "", "pts", "devpts", MS_NOSUID | MS_NOEXEC, *opts);
 			if (ret == 0)
 				break;
 		}
-
 		if (ret < 0)
 			return log_error_errno(-1, errno, "Failed to mount new devpts instance");
 
@@ -1782,9 +1779,9 @@ static int lxc_finalize_devpts_child(struct lxc_handler *handler)
 			TRACE("Failed to create detached devpts mount");
 		}
 
-		handler->conf->devpts_fd = move_fd(devpts_fd);
 		DEBUG("Mounted new devpts instance with options \"%s\"", *opts);
 	}
+	handler->conf->devpts_fd = move_fd(devpts_fd);
 
 	/* Remove any pre-existing /dev/ptmx file. */
 	ret = unlinkat(rootfs->dfd_dev, "ptmx", 0);
@@ -1801,8 +1798,8 @@ static int lxc_finalize_devpts_child(struct lxc_handler *handler)
 		return log_error_errno(-1, errno, "Failed to create \"/dev/ptmx\" file as bind mount target");
 	DEBUG("Created \"/dev/ptmx\" file as bind mount target");
 
-	/* Fallback option: create symlink /dev/ptmx -> /dev/pts/ptmx  */
-	ret = mount("/dev/pts/ptmx", "/dev/ptmx", NULL, MS_BIND, NULL);
+	/* Main option: use a bind-mount to please AppArmor  */
+	ret = mount_beneath_fd(rootfs->dfd_dev, "pts/ptmx", "ptmx", NULL, MS_BIND, NULL);
 	if (!ret)
 		return log_debug(0, "Bind mounted \"/dev/pts/ptmx\" to \"/dev/ptmx\"");
 	else
@@ -1815,7 +1812,7 @@ static int lxc_finalize_devpts_child(struct lxc_handler *handler)
 		return log_error_errno(-1, errno, "Failed to remove existing \"/dev/ptmx\"");
 
 	/* Fallback option: Create symlink /dev/ptmx -> /dev/pts/ptmx. */
-	ret = symlinkat("/dev/pts/ptmx", rootfs->dfd_dev, "/dev/ptmx");
+	ret = symlinkat("/dev/pts/ptmx", rootfs->dfd_dev, "dev/ptmx");
 	if (ret < 0)
 		return log_error_errno(-1, errno, "Failed to create symlink from \"/dev/ptmx\" to \"/dev/pts/ptmx\"");
 
@@ -2042,6 +2039,9 @@ static int lxc_setup_console(const struct lxc_handler *handler,
 		ret = lxc_setup_ttydir_console(rootfs, console, ttydir);
 	else
 		ret = lxc_setup_dev_console(rootfs, console);
+	if (ret < 0)
+		return syserror("Failed to setup console");
+
 	fd_pty = move_fd(console->pty);
 
 	/*
@@ -4071,7 +4071,7 @@ int lxc_sync_fds_parent(struct lxc_handler *handler)
 	if (ret < 0)
 		return syserror_ret(ret, "Failed to receive seccomp notify fd from child");
 
-	ret = lxc_setup_devpts_parent(handler);
+	ret = lxc_recv_devpts_from_child(handler);
 	if (ret < 0)
 		return syserror_ret(ret, "Failed to receive devpts fd from child");
 
@@ -4219,7 +4219,7 @@ int lxc_setup(struct lxc_handler *handler)
 	if (ret < 0)
 		return log_error(-1, "Failed to mount transient procfs instance for LSMs");
 
-	ret = lxc_prepare_devpts_child(handler);
+	ret = lxc_setup_devpts_child(handler);
 	if (ret < 0)
 		return log_error(-1, "Failed to prepare new devpts instance");
 
@@ -4240,10 +4240,6 @@ int lxc_setup(struct lxc_handler *handler)
 	if (lxc_conf->autodev > 0)
 		(void)lxc_setup_boot_id();
 
-	ret = lxc_finalize_devpts_child(handler);
-	if (ret < 0)
-		return log_error(-1, "Failed to setup new devpts instance");
-
 	ret = lxc_create_ttys(handler);
 	if (ret < 0)
 		return -1;

src/lxc/mount_utils.c

@@ -11,6 +11,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include "conf.h"
 #include "file_utils.h"
 #include "log.h"
 #include "macro.h"
@@ -604,3 +605,33 @@ bool can_use_bind_mounts(void)
 
 	return supported == 1;
 }
+
+int mount_beneath_fd(int fd, const char *source, const char *target,
+		     const char *fs_name, unsigned int flags, const void *data)
+{
+	int ret;
+	char buf_source[PATH_MAX], buf_target[PATH_MAX];
+
+	if (abspath(source) || abspath(target))
+		return ret_errno(EINVAL);
+
+	ret = strnprintf(buf_target, sizeof(buf_target), "/proc/self/fd/%d/%s", fd, target);
+	if (ret < 0)
+		return syserror("Failed to create path");
+
+	if (is_empty_string(source)) {
+		ret = mount(fs_name ?: "", buf_target, fs_name, flags, data);
+	} else {
+		ret = strnprintf(buf_source, sizeof(buf_source), "/proc/self/fd/%d/%s", fd, source);
+		if (ret < 0)
+			return syserror("Failed to create path");
+
+		source = buf_source;
+		ret = mount(source, buf_target, fs_name, flags, data);
+	}
+	if (ret < 0)
+		return syserror("Failed to mount \"%s\" to \"%s\"", source, buf_target);
+
+	TRACE("Mounted \"%s\" to \"%s\"", source, buf_target);
+	return 0;
+}

src/lxc/mount_utils.h

@@ -12,6 +12,8 @@
 #include "memory_utils.h"
 #include "syscall_wrappers.h"
 
+struct lxc_rootfs;
+
 /* open_tree() flags */
 
 #ifndef AT_RECURSIVE
@@ -189,7 +191,6 @@ __hidden extern int fd_bind_mount(int dfd_from, const char *path_from,
 				  int dfd_to, const char *path_to,
 				  __u64 o_flags_to, __u64 resolve_flags_to,
 				  unsigned int attr_flags, bool recursive);
-
 __hidden extern int fd_mount_idmapped(int dfd_from, const char *path_from,
 				      __u64 o_flags_from, __u64 resolve_flags_from,
 				      int dfd_to, const char *path_to,
@@ -220,5 +221,8 @@ __hidden extern unsigned long add_required_remount_flags(const char *s,
 
 __hidden extern bool can_use_mount_api(void);
 __hidden extern bool can_use_bind_mounts(void);
+__hidden extern int mount_beneath_fd(int fd, const char *source,
+				     const char *target, const char *fs_name,
+				     unsigned int flags, const void *data);
 
 #endif /* __LXC_MOUNT_UTILS_H */

src/lxc/terminal.c

@@ -909,10 +909,12 @@ static int lxc_terminal_create_foreign(struct lxc_conf *conf, struct lxc_termina
 	return -ENODEV;
 }
 
-static int lxc_terminal_create_native(const char *name, const char *lxcpath, struct lxc_conf *conf,
+static int lxc_terminal_create_native(const char *name, const char *lxcpath,
+				      struct lxc_conf *conf,
 				      struct lxc_terminal *terminal)
 {
-	__do_close int devpts_fd = -EBADF;
+	__do_close int devpts_fd = -EBADF, fd_pty = -EBADF;
+	int pty_nr = -1;
 	int ret;
 
 	devpts_fd = lxc_cmd_get_devpts_fd(name, lxcpath);
@@ -949,12 +951,31 @@ static int lxc_terminal_create_native(const char *name, const char *lxcpath, str
 		goto err;
 	}
 
-	ret = ttyname_r(terminal->pty, terminal->name, sizeof(terminal->name));
+	ret = ioctl(terminal->ptx, TIOCGPTN, &pty_nr);
 	if (ret) {
 		SYSWARN("Failed to retrieve name of terminal pty");
 		goto err;
 	}
 
+	fd_pty = open_at(devpts_fd, fdstr(pty_nr), PROTECT_OPATH_FILE,
+			PROTECT_LOOKUP_ABSOLUTE_XDEV, 0);
+	if (fd_pty < 0) {
+		SYSWARN("Failed to open terminal pty fd by path %d/%d", devpts_fd, pty_nr);
+		goto err;
+	}
+
+	if (!same_file_lax(terminal->pty, fd_pty)) {
+		SYSWARN("Terminal file descriptor changed");
+		goto err;
+	}
+
+	ret = strnprintf(terminal->name, sizeof(terminal->name), "dev/pts/%d",
+			pty_nr);
+	if (ret < 0) {
+		SYSWARN("Failed to create terminal pty name");
+		goto err;
+	}
+
 	ret = lxc_terminal_peer_default(terminal);
 	if (ret < 0) {
 		SYSWARN("Failed to allocate proxy terminal");