Merge pull request #2440 from brauner/2018-06-30/console_fixes

CODING_STYLE.md

@@ -672,3 +672,13 @@ string.
 Unless you have a valid reason to accept truncation you must check whether
 truncation has occurred, treat it as an error, and handle the error
 appropriately.
+
+#### Use `strlcat()` instead of `strncat()`
+
+When concatenating strings always use `strlcat()` instead of `strncat()`. The
+advantage of `strlcat()` is that it will always append a `\0` byte to the
+string.
+
+Unless you have a valid reason to accept truncation you must check whether
+truncation has occurred, treat it as an error, and handle the error
+appropriately.

src/lxc/attach.c

@@ -980,7 +980,6 @@ static int lxc_attach_terminal(struct lxc_conf *conf,
 			       struct lxc_terminal *terminal)
 {
 	int ret;
-	struct termios oldtios;
 
 	lxc_terminal_init(terminal);
 
@@ -990,12 +989,6 @@ static int lxc_attach_terminal(struct lxc_conf *conf,
 		return -1;
 	}
 
-	ret = lxc_setup_tios(terminal->master, &oldtios);
-	if (ret < 0) {
-		SYSERROR("Failed to setup terminal");
-		return -1;
-	}
-
 	/* Shift ttys to container. */
 	ret = lxc_terminal_map_ids(conf, terminal);
 	if (ret < 0) {

src/lxc/conf.c

@@ -1238,7 +1238,6 @@ struct lxc_device_node {
 };
 
 static const struct lxc_device_node lxc_devices[] = {
-	{ "console", S_IFCHR | S_IRWXU | S_IRWXG | S_IRWXO, 1, 5 },
 	{ "full",    S_IFCHR | S_IRWXU | S_IRWXG | S_IRWXO, 1, 7 },
 	{ "null",    S_IFCHR | S_IRWXU | S_IRWXG | S_IRWXO, 1, 3 },
 	{ "random",  S_IFCHR | S_IRWXU | S_IRWXG | S_IRWXO, 1, 8 },
@@ -1648,7 +1647,7 @@ static int setup_personality(int persona)
 static int lxc_setup_dev_console(const struct lxc_rootfs *rootfs,
 				 const struct lxc_terminal *console)
 {
-	int fd, ret;
+	int ret;
 	char path[MAXPATHLEN];
 	char *rootfs_path = rootfs->path ? rootfs->mount : "";
 
@@ -1675,17 +1674,15 @@ 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.
 	 */
-	fd = open(path, O_CREAT | O_EXCL, S_IXUSR | S_IXGRP | S_IXOTH);
-	if (fd < 0) {
+	ret = mknod(path, S_IFREG | 0000, 0);
+	if (ret < 0) {
 		if (errno != EEXIST) {
 			SYSERROR("Failed to create console");
 			return -errno;
 		}
-	} else {
-		close(fd);
 	}
 
-	ret = chmod(console->name, S_IXUSR | S_IXGRP | S_IXOTH);
+	ret = fchmod(console->slave, S_IXUSR | S_IXGRP | S_IXOTH);
 	if (ret < 0) {
 		SYSERROR("Failed to set mode \"0%o\" to \"%s\"",
 			 S_IXUSR | S_IXGRP | S_IXOTH, console->name);

src/lxc/start.c

@@ -842,20 +842,22 @@ int lxc_init(const char *name, struct lxc_handler *handler)
 	ret = lxc_terminal_map_ids(conf, &conf->console);
 	if (ret < 0) {
 		ERROR("Failed to chown console");
-		goto out_restore_sigmask;
+		goto out_delete_terminal;
 	}
 	TRACE("Chowned console");
 
 	handler->cgroup_ops = cgroup_init(handler);
 	if (!handler->cgroup_ops) {
 		ERROR("Failed to initialize cgroup driver");
-		goto out_restore_sigmask;
+		goto out_delete_terminal;
 	}
 	TRACE("Initialized cgroup driver");
 
 	INFO("Container \"%s\" is initialized", name);
 	return 0;
 
+out_delete_terminal:
+	lxc_terminal_delete(&handler->conf->console);
 out_restore_sigmask:
 	(void)pthread_sigmask(SIG_SETMASK, &handler->oldmask, NULL);
 out_delete_tty:

src/lxc/terminal.c

@@ -570,13 +570,32 @@ static int lxc_terminal_peer_proxy_alloc(struct lxc_terminal *terminal,
 	/* This is the proxy terminal that will be given to the client, and
 	 * that the real terminal master will send to / recv from.
 	 */
-	ret = openpty(&terminal->proxy.master, &terminal->proxy.slave,
-		      terminal->proxy.name, NULL, NULL);
+	ret = openpty(&terminal->proxy.master, &terminal->proxy.slave, NULL,
+		      NULL, NULL);
 	if (ret < 0) {
 		SYSERROR("Failed to open proxy terminal");
 		return -1;
 	}
 
+	ret = ttyname_r(terminal->proxy.slave, terminal->proxy.name,
+			sizeof(terminal->proxy.name));
+	if (ret < 0) {
+		SYSERROR("Failed to retrieve name of proxy terminal slave");
+		goto on_error;
+	}
+
+	ret = fd_cloexec(terminal->proxy.master, true);
+	if (ret < 0) {
+		SYSERROR("Failed to set FD_CLOEXEC flag on proxy terminal master");
+		goto on_error;
+	}
+
+	ret = fd_cloexec(terminal->proxy.slave, true);
+	if (ret < 0) {
+		SYSERROR("Failed to set FD_CLOEXEC flag on proxy terminal slave");
+		goto on_error;
+	}
+
 	ret = lxc_setup_tios(terminal->proxy.slave, &oldtermio);
 	if (ret < 0)
 		goto on_error;
@@ -862,19 +881,25 @@ int lxc_terminal_create(struct lxc_terminal *terminal)
 {
 	int ret;
 
-	ret = openpty(&terminal->master, &terminal->slave, terminal->name, NULL, NULL);
+	ret = openpty(&terminal->master, &terminal->slave, NULL, NULL, NULL);
 	if (ret < 0) {
 		SYSERROR("Failed to open terminal");
 		return -1;
 	}
 
-	ret = fcntl(terminal->master, F_SETFD, FD_CLOEXEC);
+	ret = ttyname_r(terminal->slave, terminal->name, sizeof(terminal->name));
+	if (ret < 0) {
+		SYSERROR("Failed to retrieve name of terminal slave");
+		goto err;
+	}
+
+	ret = fd_cloexec(terminal->master, true);
 	if (ret < 0) {
 		SYSERROR("Failed to set FD_CLOEXEC flag on terminal master");
 		goto err;
 	}
 
-	ret = fcntl(terminal->slave, F_SETFD, FD_CLOEXEC);
+	ret = fd_cloexec(terminal->slave, true);
 	if (ret < 0) {
 		SYSERROR("Failed to set FD_CLOEXEC flag on terminal slave");
 		goto err;

src/lxc/utils.c

@@ -2686,3 +2686,25 @@ void remove_trailing_newlines(char *l)
 	while (--p >= l && *p == '\n')
 		*p = '\0';
 }
+
+int fd_cloexec(int fd, bool cloexec)
+{
+	int oflags, nflags;
+
+	oflags = fcntl(fd, F_GETFD, 0);
+	if (oflags < 0)
+		return -errno;
+
+	if (cloexec)
+		nflags = oflags | FD_CLOEXEC;
+	else
+		nflags = oflags & ~FD_CLOEXEC;
+
+	if (nflags == oflags)
+		return 0;
+
+	if (fcntl(fd, F_SETFD, nflags) < 0)
+		return -errno;
+
+	return 0;
+}

src/lxc/utils.h

@@ -615,5 +615,6 @@ static inline pid_t lxc_raw_gettid(void)
 
 /* Set a signal the child process will receive after the parent has died. */
 extern int lxc_set_death_signal(int signal);
+extern int fd_cloexec(int fd, bool cloexec);
 
 #endif /* __LXC_UTILS_H */

src/tests/attach.c

@@ -47,13 +47,6 @@
 static const char *lsm_config_key = NULL;
 static const char *lsm_label = NULL;
 
-bool file_exists(const char *f)
-{
-	struct stat statbuf;
-
-	return stat(f, &statbuf) == 0;
-}
-
 static void test_lsm_detect(void)
 {
 	if (lsm_enabled()) {