Merge pull request #3514 from brauner/2020-08-10/fixes

src/lxc/attach.c

@@ -876,7 +876,7 @@ static int attach_child_main(struct attach_clone_payload *payload)
 	/* Make sure that the processes STDIO is correctly owned by the user that we are switching to */
 	ret = fix_stdio_permissions(new_uid);
 	if (ret)
-		WARN("Failed to ajust stdio permissions");
+		WARN("Failed to adjust stdio permissions");
 
 	if (!lxc_switch_uid_gid(new_uid, new_gid))
 		goto on_error;
@@ -896,23 +896,11 @@ static int lxc_attach_terminal(const char *name, const char *lxcpath, struct lxc
 
 	lxc_terminal_init(terminal);
 
-	ret = lxc_terminal_create(name, lxcpath, terminal);
+	ret = lxc_terminal_create(name, lxcpath, conf, terminal);
 	if (ret < 0)
 		return log_error(-1, "Failed to create terminal");
 
-	/* Shift ttys to container. */
-	ret = lxc_terminal_map_ids(conf, terminal);
-	if (ret < 0) {
-		ERROR("Failed to chown terminal");
-		goto on_error;
-	}
-
 	return 0;
-
-on_error:
-	lxc_terminal_delete(terminal);
-	lxc_terminal_conf_free(terminal);
-	return -1;
 }
 
 static int lxc_attach_terminal_mainloop_init(struct lxc_terminal *terminal,

src/lxc/conf.c

@@ -1610,15 +1610,15 @@ static int lxc_setup_dev_console(const struct lxc_rootfs *rootfs,
 	if (!wants_console(console))
 		return 0;
 
-	ret = snprintf(path, sizeof(path), "%s/dev/console", rootfs_path);
-	if (ret < 0 || (size_t)ret >= sizeof(path))
-		return -1;
-
 	/*
 	 * When we are asked to setup a console we remove any previous
 	 * /dev/console bind-mounts.
 	 */
-	if (file_exists(path)) {
+	if (exists_file_at(rootfs->dev_mntpt_fd, "console")) {
+		ret = snprintf(path, sizeof(path), "%s/dev/console", rootfs_path);
+		if (ret < 0 || (size_t)ret >= sizeof(path))
+			return -1;
+
 		ret = lxc_unstack_mountpoint(path, false);
 		if (ret < 0)
 			return log_error_errno(-ret, errno, "Failed to unmount \"%s\"", path);
@@ -1630,7 +1630,7 @@ static int lxc_setup_dev_console(const struct lxc_rootfs *rootfs,
 	 * For unprivileged containers autodev or automounts will already have
 	 * taken care of creating /dev/console.
 	 */
-	ret = mknod(path, S_IFREG | 0000, 0);
+	ret = mknodat(rootfs->dev_mntpt_fd, "console", S_IFREG | 0000, 0);
 	if (ret < 0 && errno != EEXIST)
 		return log_error_errno(-errno, errno, "Failed to create console");
 
@@ -1639,7 +1639,7 @@ static int lxc_setup_dev_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);
 
 	if (pty_mnt_fd >= 0) {
-		ret = move_mount(pty_mnt_fd, "", -EBADF, path, MOVE_MOUNT_F_EMPTY_PATH);
+		ret = move_mount(pty_mnt_fd, "", rootfs->dev_mntpt_fd, "console", MOVE_MOUNT_F_EMPTY_PATH);
 		if (!ret) {
 			DEBUG("Moved mount \"%s\" onto \"%s\"", console->name, path);
 			goto finish;
@@ -1651,9 +1651,18 @@ static int lxc_setup_dev_console(const struct lxc_rootfs *rootfs,
 					       pty_mnt_fd, console->name, path);
 	}
 
-	ret = safe_mount(console->name, path, "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, path);
+	ret = safe_mount_beneath_at(rootfs->dev_mntpt_fd, console->name, "console", NULL, MS_BIND, NULL);
+	if (ret < 0) {
+		if (errno == ENOSYS) {
+			ret = snprintf(path, sizeof(path), "%s/dev/console", rootfs_path);
+			if (ret < 0 || (size_t)ret >= sizeof(path))
+				return -1;
+
+			ret = safe_mount(console->name, path, "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, path);
+		}
+	}
 
 finish:
 	DEBUG("Mounted pty device %d(%s) onto \"%s\"", pty_mnt_fd, console->name, path);
@@ -2581,6 +2590,7 @@ struct lxc_conf *lxc_conf_init(void)
 	}
 	new->rootfs.managed = true;
 	new->rootfs.mntpt_fd = -EBADF;
+	new->rootfs.dev_mntpt_fd = -EBADF;
 	new->logfd = -1;
 	lxc_list_init(&new->cgroup);
 	lxc_list_init(&new->cgroup2);
@@ -3259,6 +3269,11 @@ int lxc_setup(struct lxc_handler *handler)
 			return log_error(-1, "Failed to mount \"/dev\"");
 	}
 
+	lxc_conf->rootfs.dev_mntpt_fd = openat(lxc_conf->rootfs.mntpt_fd, "dev",
+						O_RDONLY | O_CLOEXEC | O_DIRECTORY | O_NOFOLLOW);
+	if (lxc_conf->rootfs.dev_mntpt_fd < 0 && errno != ENOENT)
+		return log_error_errno(-errno, errno, "Failed to open \"/dev\"");
+
 	/* Do automatic mounts (mainly /proc and /sys), but exclude those that
 	 * need to wait until other stuff has finished.
 	 */
@@ -3378,7 +3393,8 @@ int lxc_setup(struct lxc_handler *handler)
 		return log_error(-1, "Failed to drop capabilities");
 	}
 
-	close_prot_errno_disarm(lxc_conf->rootfs.mntpt_fd);
+	close_prot_errno_disarm(lxc_conf->rootfs.mntpt_fd)
+	close_prot_errno_disarm(lxc_conf->rootfs.dev_mntpt_fd)
 	NOTICE("The container \"%s\" is set up", name);
 
 	return 0;
@@ -3743,6 +3759,7 @@ void lxc_conf_free(struct lxc_conf *conf)
 	free(conf->rootfs.path);
 	free(conf->rootfs.data);
 	close_prot_errno_disarm(conf->rootfs.mntpt_fd);
+	close_prot_errno_disarm(conf->rootfs.dev_mntpt_fd);
 	free(conf->logfile);
 	if (conf->logfd != -1)
 		close(conf->logfd);
@@ -4542,7 +4559,7 @@ int userns_exec_mapped_root(const char *path, int path_fd,
 
 		ret = fchown(target_fd, 0, st.st_gid);
 		if (ret) {
-			SYSERROR("Failed to chown %d(%s) to -1:%d", target_fd, path, st.st_gid);
+			SYSERROR("Failed to chown %d(%s) to 0:%d", target_fd, path, st.st_gid);
 			_exit(EXIT_FAILURE);
 		}
 

src/lxc/conf.h

@@ -139,17 +139,19 @@ struct lxc_tty_info {
 
 /* Defines a structure to store the rootfs location, the
  * optionals pivot_root, rootfs mount paths
- * @path       : the rootfs source (directory or device)
- * @mount      : where it is mounted
- * @bev_type   : optional backing store type
- * @options    : mount options
- * @mountflags : the portion of @options that are flags
- * @data       : the portion of @options that are not flags
- * @managed    : whether it is managed by LXC
- * @mntpt_fd   : fd for @mount
+ * @path         : the rootfs source (directory or device)
+ * @mount        : where it is mounted
+ * @bev_type     : optional backing store type
+ * @options      : mount options
+ * @mountflags   : the portion of @options that are flags
+ * @data         : the portion of @options that are not flags
+ * @managed      : whether it is managed by LXC
+ * @mntpt_fd	 : fd for @mount
+ * @dev_mntpt_fd : fd for /dev of the container
  */
 struct lxc_rootfs {
 	int mntpt_fd;
+	int dev_mntpt_fd;
 	char *path;
 	char *mount;
 	char *bdev_type;

src/lxc/file_utils.c

@@ -18,6 +18,7 @@
 #include "macro.h"
 #include "memory_utils.h"
 #include "string_utils.h"
+#include "syscall_wrappers.h"
 #include "utils.h"
 
 int lxc_open_dirfd(const char *dir)
@@ -551,3 +552,28 @@ bool exists_dir_at(int dir_fd, const char *path)
 
 	return S_ISDIR(sb.st_mode);
 }
+
+bool exists_file_at(int dir_fd, const char *path)
+{
+	struct stat sb;
+
+	return fstatat(dir_fd, path, &sb, 0) == 0;
+}
+
+int open_beneath(int dir_fd, const char *path, unsigned int flags)
+{
+	__do_close int fd = -EBADF;
+	struct lxc_open_how how = {
+		.flags		= flags,
+		.resolve	= RESOLVE_NO_XDEV | RESOLVE_NO_SYMLINKS | RESOLVE_NO_MAGICLINKS | RESOLVE_BENEATH,
+	};
+
+	fd = openat2(dir_fd, path, &how, sizeof(how));
+	if (fd >= 0)
+		return move_fd(fd);
+
+	if (errno != ENOSYS)
+		return -errno;
+
+	return openat(dir_fd, path, O_NOFOLLOW | flags);
+}

src/lxc/file_utils.h

@@ -74,5 +74,7 @@ __hidden extern FILE *fdopen_cached(int fd, const char *mode, void **caller_free
 __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);
 
 #endif /* __LXC_FILE_UTILS_H */

src/lxc/start.c

@@ -815,13 +815,6 @@ int lxc_init(const char *name, struct lxc_handler *handler)
 	}
 	TRACE("Created console");
 
-	ret = lxc_terminal_map_ids(conf, &conf->console);
-	if (ret < 0) {
-		ERROR("Failed to chown console");
-		goto out_delete_terminal;
-	}
-	TRACE("Chowned console");
-
 	handler->cgroup_ops = cgroup_init(handler->conf);
 	if (!handler->cgroup_ops) {
 		ERROR("Failed to initialize cgroup driver");

src/lxc/terminal.c

@@ -828,7 +828,31 @@ int lxc_terminal_create_log_file(struct lxc_terminal *terminal)
 	return 0;
 }
 
-static int lxc_terminal_create_foreign(struct lxc_terminal *terminal)
+static int lxc_terminal_map_ids(struct lxc_conf *c, struct lxc_terminal *terminal)
+{
+	int ret;
+
+	if (lxc_list_empty(&c->id_map))
+		return 0;
+
+	if (is_empty_string(terminal->name) && terminal->pty < 0)
+		return 0;
+
+	if (terminal->pty >= 0)
+		ret = userns_exec_mapped_root(NULL, terminal->pty, c);
+	else
+		ret = userns_exec_mapped_root(terminal->name, -EBADF, c);
+	if (ret < 0)
+		return log_error(-1, "Failed to chown terminal %d(%s)", terminal->pty,
+				 !is_empty_string(terminal->name) ? terminal->name : "(null)");
+
+	TRACE("Chowned terminal %d(%s)", terminal->pty,
+	      !is_empty_string(terminal->name) ? terminal->name : "(null)");
+
+	return 0;
+}
+
+static int lxc_terminal_create_foreign(struct lxc_conf *conf, struct lxc_terminal *terminal)
 {
 	int ret;
 
@@ -838,6 +862,12 @@ static int lxc_terminal_create_foreign(struct lxc_terminal *terminal)
 		return -1;
 	}
 
+	ret = lxc_terminal_map_ids(conf, terminal);
+	if (ret < 0) {
+		SYSERROR("Failed to change ownership of terminal multiplexer device");
+		goto err;
+	}
+
 	ret = ttyname_r(terminal->pty, terminal->name, sizeof(terminal->name));
 	if (ret < 0) {
 		SYSERROR("Failed to retrieve name of terminal pty");
@@ -869,38 +899,40 @@ err:
 	return -ENODEV;
 }
 
-static int lxc_terminal_create_native(const char *name, const char *lxcpath,
+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, ptx_fd = -EBADF, pty_fd = -EBADF;
+	__do_close int devpts_fd = -EBADF;
 	int ret;
 
 	devpts_fd = lxc_cmd_get_devpts_fd(name, lxcpath);
 	if (devpts_fd < 0)
 		return log_error_errno(-1, errno, "Failed to receive devpts fd");
 
-	ptx_fd = openat(devpts_fd, "ptmx", O_RDWR | O_NOCTTY | O_CLOEXEC);
-	if (ptx_fd < 0)
+	terminal->ptx = open_beneath(devpts_fd, "ptmx", O_RDWR | O_NOCTTY | O_CLOEXEC);
+	if (terminal->ptx < 0)
 		return log_error_errno(-1, errno, "Failed to open terminal multiplexer device");
 
-	ret = grantpt(ptx_fd);
-	if (ret < 0)
-		return log_error_errno(-1, errno, "Failed to grant access to multiplexer device");
+	ret = unlockpt(terminal->ptx);
+	if (ret < 0) {
+		SYSERROR("Failed to unlock multiplexer device device");
+		goto err;
+	}
 
-	ret = unlockpt(ptx_fd);
-	if (ret < 0)
-		return log_error_errno(-1, errno, "Failed to unlock multiplexer device device");
+	terminal->pty = ioctl(terminal->ptx, TIOCGPTPEER, O_RDWR | O_NOCTTY | O_CLOEXEC);
+	if (terminal->pty < 0) {
+		SYSERROR("Failed to allocate new pty device");
+		goto err;
+	}
 
-	pty_fd = ioctl(ptx_fd, TIOCGPTPEER, O_RDWR | O_NOCTTY | O_CLOEXEC);
-	if (pty_fd < 0)
-		return log_error_errno(-1, errno, "Failed to allocate new pty device");
+	// ret = lxc_terminal_map_ids(conf, terminal);
 
 	ret = ttyname_r(terminal->pty, terminal->name, sizeof(terminal->name));
-	if (ret < 0)
-		return log_error_errno(-1, errno, "Failed to retrieve name of terminal pty");
+	if (ret < 0) {
+		SYSERROR("Failed to retrieve name of terminal pty");
+		goto err;
+	}
 
-	terminal->ptx = move_fd(ptx_fd);
-	terminal->pty = move_fd(pty_fd);
 	ret = lxc_terminal_peer_default(terminal);
 	if (ret < 0) {
 		ERROR("Failed to allocate proxy terminal");
@@ -914,12 +946,13 @@ err:
 	return -ENODEV;
 }
 
-int lxc_terminal_create(const char *name, const char *lxcpath, struct lxc_terminal *terminal)
+int lxc_terminal_create(const char *name, const char *lxcpath, struct lxc_conf *conf,
+			struct lxc_terminal *terminal)
 {
-	if (!lxc_terminal_create_native(name, lxcpath, terminal))
+	if (!lxc_terminal_create_native(name, lxcpath, conf, terminal))
 		return 0;
 
-	return lxc_terminal_create_foreign(terminal);
+	return lxc_terminal_create_foreign(conf, terminal);
 }
 
 int lxc_terminal_setup(struct lxc_conf *conf)
@@ -932,7 +965,7 @@ int lxc_terminal_setup(struct lxc_conf *conf)
 		return 0;
 	}
 
-	ret = lxc_terminal_create_foreign(terminal);
+	ret = lxc_terminal_create_foreign(conf, terminal);
 	if (ret < 0)
 		return -1;
 
@@ -1209,24 +1242,3 @@ void lxc_terminal_conf_free(struct lxc_terminal *terminal)
 		lxc_ringbuf_release(&terminal->ringbuf);
 	lxc_terminal_signal_fini(terminal);
 }
-
-int lxc_terminal_map_ids(struct lxc_conf *c, struct lxc_terminal *terminal)
-{
-	int ret;
-
-	if (lxc_list_empty(&c->id_map))
-		return 0;
-
-	if (strcmp(terminal->name, "") == 0)
-		return 0;
-
-	ret = userns_exec_mapped_root(terminal->name, terminal->pty, c);
-	if (ret < 0) {
-		return log_error(-1, "Failed to chown terminal %d(%s)",
-				 terminal->pty, terminal->name);
-	}
-
-	TRACE("Chowned terminal %d(%s)", terminal->pty, terminal->name);
-
-	return 0;
-}

src/lxc/terminal.h

@@ -111,6 +111,7 @@ __hidden extern int  lxc_terminal_allocate(struct lxc_conf *conf, int sockfd, in
  *   (Handlers for SIGWINCH and I/O are not registered in a mainloop.)
  */
 __hidden extern int lxc_terminal_create(const char *name, const char *lxcpath,
+					struct lxc_conf *conf,
 					struct lxc_terminal *console);
 
 /**
@@ -250,6 +251,5 @@ __hidden extern int lxc_terminal_prepare_login(int fd);
 __hidden extern void lxc_terminal_conf_free(struct lxc_terminal *terminal);
 __hidden extern void lxc_terminal_info_init(struct lxc_terminal_info *terminal);
 __hidden extern void lxc_terminal_init(struct lxc_terminal *terminal);
-__hidden extern int lxc_terminal_map_ids(struct lxc_conf *c, struct lxc_terminal *terminal);
 
 #endif /* __LXC_TERMINAL_H */