Merge pull request #2061 from brauner/2017-12-23/attach_allocate_pty_in_api

doc/lxc-attach.sgml.in

@@ -58,7 +58,6 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
       <arg choice="opt">-R, --remount-sys-proc</arg>
       <arg choice="opt">--keep-env</arg>
       <arg choice="opt">--clear-env</arg>
-      <arg choice="opt">-L, --pty-log <replaceable>file</replaceable></arg>
       <arg choice="opt">-v, --set-var <replaceable>variable</replaceable></arg>
       <arg choice="opt">--keep-var <replaceable>variable</replaceable></arg>
       <arg choice="opt">-- <replaceable>command</replaceable></arg>
@@ -258,22 +257,6 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
       <varlistentry>
 	<term>
-	  <option>-L, --pty-log <replaceable>file</replaceable></option>
-	</term>
-	<listitem>
-	  <para>
-	Specify a file where the output of <command>lxc-attach</command> will be
-	logged.
-	  </para>
-	  <para>
-	    <emphasis>Important:</emphasis>  When a standard file descriptor
-	    does not refer to a pty output produced on it will not be logged.
-	  </para>
-	</listitem>
-      </varlistentry>
-
-      <varlistentry>
-	<term>
 	  <option>-v, --set-var <replaceable>variable</replaceable></option>
 	</term>
 	<listitem>

src/lxc/attach.c

@@ -61,10 +61,12 @@
 #include "conf.h"
 #include "config.h"
 #include "confile.h"
+#include "console.h"
 #include "log.h"
 #include "lsm/lsm.h"
 #include "lxclock.h"
 #include "lxcseccomp.h"
+#include "mainloop.h"
 #include "namespace.h"
 #include "utils.h"
 
@@ -259,10 +261,16 @@ static inline void lxc_proc_close_ns_fd(struct lxc_proc_context_info *ctx)
 static void lxc_proc_put_context_info(struct lxc_proc_context_info *ctx)
 {
 	free(ctx->lsm_label);
-	if (ctx->container)
+	ctx->lsm_label = NULL;
+
+	if (ctx->container) {
 		lxc_container_put(ctx->container);
+		ctx->container = NULL;
+	}
+
 	lxc_proc_close_ns_fd(ctx);
 	free(ctx);
+	ctx = NULL;
 }
 
 /**
@@ -812,22 +820,35 @@ static signed long get_personality(const char *name, const char *lxcpath)
 
 struct attach_clone_payload {
 	int ipc_socket;
+	int pty_fd;
 	lxc_attach_options_t *options;
 	struct lxc_proc_context_info *init_ctx;
 	lxc_attach_exec_t exec_function;
 	void *exec_payload;
 };
 
+static void lxc_put_attach_clone_payload(struct attach_clone_payload *p)
+{
+	if (p->ipc_socket >= 0) {
+		shutdown(p->ipc_socket, SHUT_RDWR);
+		close(p->ipc_socket);
+		p->ipc_socket = -EBADF;
+	}
+
+	if (p->pty_fd >= 0) {
+		close(p->pty_fd);
+		p->pty_fd = -EBADF;
+	}
+
+	if (p->init_ctx)
+		lxc_proc_put_context_info(p->init_ctx);
+}
+
 static int attach_child_main(struct attach_clone_payload *payload)
 {
 	int fd, lsm_fd, ret;
-	long flags;
-#if HAVE_SYS_PERSONALITY_H
-	long new_personality;
-#endif
 	uid_t new_uid;
 	gid_t new_gid;
-	int ipc_socket = payload->ipc_socket;
 	lxc_attach_options_t* options = payload->options;
 	struct lxc_proc_context_info* init_ctx = payload->init_ctx;
 	bool needs_lsm = (options->namespaces & CLONE_NEWNS) &&
@@ -842,36 +863,32 @@ static int attach_child_main(struct attach_clone_payload *payload)
 	if (!(options->namespaces & CLONE_NEWNS) &&
 	    (options->attach_flags & LXC_ATTACH_REMOUNT_PROC_SYS)) {
 		ret = lxc_attach_remount_sys_proc();
-		if (ret < 0) {
-			shutdown(ipc_socket, SHUT_RDWR);
-			rexit(-1);
-		}
+		if (ret < 0)
+			goto on_error;
+		TRACE("Remounted \"/proc\" and \"/sys\"");
 	}
 
-	/* Now perform additional attachments. */
+/* Now perform additional attachments. */
 #if HAVE_SYS_PERSONALITY_H
+	if (options->attach_flags & LXC_ATTACH_SET_PERSONALITY) {
+		long new_personality;
+
 		if (options->personality < 0)
 			new_personality = init_ctx->personality;
 		else
 			new_personality = options->personality;
-
-	if (options->attach_flags & LXC_ATTACH_SET_PERSONALITY) {
 		ret = personality(new_personality);
-		if (ret < 0) {
-			SYSERROR("Could not ensure correct architecture");
-			shutdown(ipc_socket, SHUT_RDWR);
-			rexit(-1);
-		}
+		if (ret < 0)
+			goto on_error;
+		TRACE("Set new personality");
 	}
 #endif
 
 	if (options->attach_flags & LXC_ATTACH_DROP_CAPABILITIES) {
 		ret = lxc_attach_drop_privs(init_ctx);
-		if (ret < 0) {
-			ERROR("Could not drop privileges");
-			shutdown(ipc_socket, SHUT_RDWR);
-			rexit(-1);
-		}
+		if (ret < 0)
+			goto on_error;
+		TRACE("Dropped capabilities");
 	}
 
 	/* Always set the environment (specify (LXC_ATTACH_KEEP_ENV, NULL, NULL)
@@ -880,11 +897,9 @@ static int attach_child_main(struct attach_clone_payload *payload)
 	ret = lxc_attach_set_environment(options->env_policy,
 					 options->extra_env_vars,
 					 options->extra_keep_env);
-	if (ret < 0) {
-		ERROR("Failed to set initial environment for attached process");
-		shutdown(ipc_socket, SHUT_RDWR);
-		rexit(-1);
-	}
+	if (ret < 0)
+		goto on_error;
+	TRACE("Set up environment");
 
 	/* This remark only affects fully unprivileged containers:
 	 * Receive fd for LSM security module before we set{g,u}id(). The reason
@@ -898,14 +913,18 @@ static int attach_child_main(struct attach_clone_payload *payload)
 	 * set{g,u}id().
 	 */
 	if (needs_lsm) {
-		ret = lxc_abstract_unix_recv_fds(ipc_socket, &lsm_fd, 1, NULL, 0);
-		if (ret <= 0) {
-			shutdown(ipc_socket, SHUT_RDWR);
-			rexit(-1);
-		}
+		ret = lxc_abstract_unix_recv_fds(payload->ipc_socket, &lsm_fd, 1, NULL, 0);
+		if (ret <= 0)
+			goto on_error;
 		TRACE("Received LSM label file descriptor %d from parent", lsm_fd);
 	}
 
+	if (options->stdin_fd > 0 && isatty(options->stdin_fd)) {
+		ret = lxc_make_controlling_pty(options->stdin_fd);
+		if (ret < 0)
+			goto on_error;
+	}
+
 	/* Set {u,g}id. */
 	new_uid = 0;
 	new_gid = 0;
@@ -920,48 +939,24 @@ static int attach_child_main(struct attach_clone_payload *payload)
 	if (options->gid != (gid_t)-1)
 		new_gid = options->gid;
 
-	/* Setup the controlling tty. */
-	if (options->stdin_fd && isatty(options->stdin_fd)) {
-		if (setsid() < 0) {
-			SYSERROR("Unable to setsid.");
-			shutdown(ipc_socket, SHUT_RDWR);
-			rexit(-1);
-		}
-
-		if (ioctl(options->stdin_fd, TIOCSCTTY, (char *)NULL) < 0) {
-			SYSERROR("Unable to set TIOCSTTY.");
-			shutdown(ipc_socket, SHUT_RDWR);
-			rexit(-1);
-		}
-	}
-
 	/* Try to set the {u,g}id combination. */
-	if ((new_gid != 0 || options->namespaces & CLONE_NEWUSER)) {
-		if (setgid(new_gid) || setgroups(0, NULL)) {
-			SYSERROR("Switching to container gid.");
-			shutdown(ipc_socket, SHUT_RDWR);
-			rexit(-1);
-		}
-	}
-	if ((new_uid != 0 || options->namespaces & CLONE_NEWUSER) &&
-	    setuid(new_uid)) {
-		SYSERROR("Switching to container uid.");
-		shutdown(ipc_socket, SHUT_RDWR);
-		rexit(-1);
+	if (new_uid != 0 || new_gid != 0 || options->namespaces & CLONE_NEWUSER) {
+		ret = lxc_switch_uid_gid(new_uid, new_gid);
+		if (ret < 0)
+			goto on_error;
+
+		ret = lxc_setgroups(0, NULL);
+		if (ret < 0)
+			goto on_error;
 	}
 
 	if ((init_ctx->container && init_ctx->container->lxc_conf &&
 	     init_ctx->container->lxc_conf->no_new_privs) ||
 	    (options->attach_flags & LXC_ATTACH_NO_NEW_PRIVS)) {
-		if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0) {
-			SYSERROR("PR_SET_NO_NEW_PRIVS could not be set. "
-				 "Process can use execve() gainable "
-				 "privileges.");
-			shutdown(ipc_socket, SHUT_RDWR);
-			rexit(-1);
-		}
-		INFO("PR_SET_NO_NEW_PRIVS is set. Process cannot use execve() "
-		     "gainable privileges.");
+		ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
+		if (ret < 0)
+			goto on_error;
+		TRACE("Set PR_SET_NO_NEW_PRIVS");
 	}
 
 	if (needs_lsm) {
@@ -970,25 +965,22 @@ static int attach_child_main(struct attach_clone_payload *payload)
 		/* Change into our new LSM profile. */
 		on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ? 1 : 0;
 		ret = lsm_set_label_at(lsm_fd, on_exec, init_ctx->lsm_label);
-		if (ret < 0) {
-			SYSERROR("Failed to set LSM label.");
-			shutdown(ipc_socket, SHUT_RDWR);
-			close(lsm_fd);
-			rexit(-1);
-		}
 		close(lsm_fd);
+		if (ret < 0)
+			goto on_error;
+		TRACE("Set LSM label");
 	}
 
 	if (init_ctx->container && init_ctx->container->lxc_conf &&
-	    init_ctx->container->lxc_conf->seccomp &&
-	    (lxc_seccomp_load(init_ctx->container->lxc_conf) != 0)) {
-		ERROR("Failed to load seccomp policy.");
-		shutdown(ipc_socket, SHUT_RDWR);
-		rexit(-1);
+	    init_ctx->container->lxc_conf->seccomp) {
+		ret = lxc_seccomp_load(init_ctx->container->lxc_conf);
+		if (ret < 0)
+			goto on_error;
+		TRACE("Loaded seccomp profile");
 	}
-
-	shutdown(ipc_socket, SHUT_RDWR);
-	close(ipc_socket);
+	shutdown(payload->ipc_socket, SHUT_RDWR);
+	close(payload->ipc_socket);
+	payload->ipc_socket = -EBADF;
 	lxc_proc_put_context_info(init_ctx);
 
 	/* The following is done after the communication socket is shut down.
@@ -1000,35 +992,144 @@ static int attach_child_main(struct attach_clone_payload *payload)
 	/* Fd handling for stdin, stdout and stderr; ignore errors here, user
 	 * may want to make sure the fds are closed, for example.
 	 */
-	if (options->stdin_fd >= 0 && options->stdin_fd != 0)
-		dup2(options->stdin_fd, 0);
-	if (options->stdout_fd >= 0 && options->stdout_fd != 1)
-		dup2(options->stdout_fd, 1);
-	if (options->stderr_fd >= 0 && options->stderr_fd != 2)
-		dup2(options->stderr_fd, 2);
+	if (options->stdin_fd >= 0 && options->stdin_fd != STDIN_FILENO)
+		dup2(options->stdin_fd, STDIN_FILENO);
+
+	if (options->stdout_fd >= 0 && options->stdout_fd != STDOUT_FILENO)
+		dup2(options->stdout_fd, STDOUT_FILENO);
+
+	if (options->stderr_fd >= 0 && options->stderr_fd != STDERR_FILENO)
+		dup2(options->stderr_fd, STDERR_FILENO);
 
 	/* close the old fds */
-	if (options->stdin_fd > 2)
+	if (options->stdin_fd > STDERR_FILENO)
 		close(options->stdin_fd);
-	if (options->stdout_fd > 2)
+
+	if (options->stdout_fd > STDERR_FILENO)
 		close(options->stdout_fd);
-	if (options->stderr_fd > 2)
+
+	if (options->stderr_fd > STDERR_FILENO)
 		close(options->stderr_fd);
 
 	/* Try to remove FD_CLOEXEC flag from stdin/stdout/stderr, but also
 	 * here, ignore errors.
 	 */
-	for (fd = 0; fd <= 2; fd++) {
+	for (fd = STDIN_FILENO; fd <= STDERR_FILENO; fd++) {
+		int flags;
+
 		flags = fcntl(fd, F_GETFL);
 		if (flags < 0)
 			continue;
-		if (flags & FD_CLOEXEC)
-			if (fcntl(fd, F_SETFL, flags & ~FD_CLOEXEC) < 0)
-				SYSERROR("Unable to clear FD_CLOEXEC from file descriptor.");
+
+		if ((flags & FD_CLOEXEC) == 0)
+			continue;
+
+		ret = fcntl(fd, F_SETFL, flags & ~FD_CLOEXEC);
+		if (ret < 0) {
+			SYSERROR("Failed to clear FD_CLOEXEC from file descriptor %d", fd);
+			goto on_error;
+		}
+	}
+
+	if (options->attach_flags & LXC_ATTACH_ALLOCATE_PTY) {
+		ret = lxc_login_pty(payload->pty_fd);
+		if (ret < 0) {
+			SYSERROR("Failed to prepare pty file descriptor %d", payload->pty_fd);
+			goto on_error;
+		}
+		TRACE("Prepared pty file descriptor %d", payload->pty_fd);
 	}
 
 	/* We're done, so we can now do whatever the user intended us to do. */
 	rexit(payload->exec_function(payload->exec_payload));
+
+on_error:
+	lxc_put_attach_clone_payload(payload);
+	rexit(EXIT_FAILURE);
+}
+
+static int lxc_attach_pty(struct lxc_conf *conf, struct lxc_console *pty)
+{
+	int ret;
+
+	lxc_pty_init(pty);
+
+	ret = lxc_pty_create(pty);
+	if (ret < 0) {
+		SYSERROR("Failed to create pty");
+		return -1;
+	}
+
+	/* Shift ttys to container. */
+	ret = lxc_pty_map_ids(conf, pty);
+	if (ret < 0) {
+		ERROR("Failed to shift pty");
+		goto on_error;
+	}
+
+	return 0;
+
+on_error:
+	lxc_console_delete(pty);
+	lxc_pty_conf_free(pty);
+	return -1;
+}
+
+static int lxc_attach_pty_mainloop_init(struct lxc_console *pty,
+					struct lxc_epoll_descr *descr)
+{
+	int ret;
+
+	ret = lxc_mainloop_open(descr);
+	if (ret < 0) {
+		ERROR("Failed to create mainloop");
+		return -1;
+	}
+
+	ret = lxc_console_mainloop_add(descr, pty);
+	if (ret < 0) {
+		ERROR("Failed to add handlers to mainloop");
+		lxc_mainloop_close(descr);
+		return -1;
+	}
+
+	return 0;
+}
+
+static inline void lxc_attach_pty_close_master(struct lxc_console *pty)
+{
+	if (pty->master < 0)
+		return;
+
+	close(pty->master);
+	pty->master = -EBADF;
+}
+
+static inline void lxc_attach_pty_close_slave(struct lxc_console *pty)
+{
+	if (pty->slave < 0)
+		return;
+
+	close(pty->slave);
+	pty->slave = -EBADF;
+}
+
+static inline void lxc_attach_pty_close_peer(struct lxc_console *pty)
+{
+	if (pty->peer < 0)
+		return;
+
+	close(pty->peer);
+	pty->peer = -EBADF;
+}
+
+static inline void lxc_attach_pty_close_log(struct lxc_console *pty)
+{
+	if (pty->log_fd < 0)
+		return;
+
+	close(pty->log_fd);
+	pty->log_fd = -EBADF;
 }
 
 int lxc_attach(const char *name, const char *lxcpath,
@@ -1039,8 +1140,9 @@ int lxc_attach(const char *name, const char *lxcpath,
 	int ipc_sockets[2];
 	char *cwd, *new_cwd;
 	signed long personality;
-	pid_t attached_pid, expected, init_pid, pid;
+	pid_t attached_pid, init_pid, pid;
 	struct lxc_proc_context_info *init_ctx;
+	struct lxc_console pty;
 	struct attach_clone_payload payload = {0};
 
 	ret = access("/proc/self/ns", X_OK);
@@ -1157,6 +1259,18 @@ int lxc_attach(const char *name, const char *lxcpath,
 		return -1;
 	}
 
+	if (options->attach_flags & LXC_ATTACH_ALLOCATE_PTY) {
+		ret = lxc_attach_pty(init_ctx->container->lxc_conf, &pty);
+		if (ret < 0) {
+			ERROR("Failed to allocate pty");
+			free(cwd);
+			lxc_proc_put_context_info(init_ctx);
+			return -1;
+		}
+
+		pty.log_fd = options->log_fd;
+	}
+
 	/* Create a socket pair for IPC communication; set SOCK_CLOEXEC in order
 	 * to make sure we don't irritate other threads that want to fork+exec
 	 * away
@@ -1217,13 +1331,16 @@ int lxc_attach(const char *name, const char *lxcpath,
 	}
 
 	if (pid) {
+		int ret_parent = -1;
 		pid_t to_cleanup_pid = pid;
+		struct lxc_epoll_descr descr = {0};
 
-		/* Initial thread, we close the socket that is for the
-		 * subprocesses.
-		 */
+		/* close unneeded file descriptors */
 		close(ipc_sockets[1]);
 		free(cwd);
+		lxc_proc_close_ns_fd(init_ctx);
+		if (options->attach_flags & LXC_ATTACH_ALLOCATE_PTY)
+			lxc_attach_pty_close_slave(&pty);
 
 		/* Attach to cgroup, if requested. */
 		if (options->attach_flags & LXC_ATTACH_MOVE_TO_CGROUP) {
@@ -1234,21 +1351,30 @@ int lxc_attach(const char *name, const char *lxcpath,
 		}
 
 		/* Setup resource limits */
-		if (!lxc_list_empty(&init_ctx->container->lxc_conf->limits))
-			if (setup_resource_limits(&init_ctx->container->lxc_conf->limits, pid) < 0)
+		if (!lxc_list_empty(&init_ctx->container->lxc_conf->limits)) {
+			ret = setup_resource_limits(&init_ctx->container->lxc_conf->limits, pid);
+			if (ret < 0)
+				goto on_error;
+		}
+
+		if (options->attach_flags & LXC_ATTACH_ALLOCATE_PTY) {
+			ret = lxc_attach_pty_mainloop_init(&pty, &descr);
+			if (ret < 0)
 				goto on_error;
+			TRACE("Initalized pty mainloop");
+		}
 
 		/* Let the child process know to go ahead. */
 		status = 0;
 		ret = lxc_write_nointr(ipc_sockets[0], &status, sizeof(status));
 		if (ret != sizeof(status))
-			goto on_error;
+			goto close_mainloop;
 		TRACE("Told intermediate process to start initializing");
 
 		/* Get pid of attached process from intermediate process. */
 		ret = lxc_read_nointr(ipc_sockets[0], &attached_pid, sizeof(attached_pid));
 		if (ret != sizeof(attached_pid))
-			goto on_error;
+			goto close_mainloop;
 		TRACE("Received pid %d of attached process in parent pid namespace", attached_pid);
 
 		/* Ignore SIGKILL (CTRL-C) and SIGQUIT (CTRL-\) - issue #313. */
@@ -1260,7 +1386,7 @@ int lxc_attach(const char *name, const char *lxcpath,
 		/* Reap intermediate process. */
 		ret = wait_for_pid(pid);
 		if (ret < 0)
-			goto on_error;
+			goto close_mainloop;
 		TRACE("Intermediate process %d exited", pid);
 
 		/* We will always have to reap the attached process now. */
@@ -1276,7 +1402,7 @@ int lxc_attach(const char *name, const char *lxcpath,
 			on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ? 1 : 0;
 			labelfd = lsm_open(attached_pid, on_exec);
 			if (labelfd < 0)
-				goto on_error;
+				goto close_mainloop;
 			TRACE("Opened LSM label file descriptor %d", labelfd);
 
 			/* Send child fd of the LSM security module to write to. */
@@ -1284,45 +1410,66 @@ int lxc_attach(const char *name, const char *lxcpath,
 			close(labelfd);
 			if (ret <= 0) {
 				SYSERROR("%d", (int)ret);
-				goto on_error;
+				goto close_mainloop;
 			}
 			TRACE("Sent LSM label file descriptor %d to child", labelfd);
 		}
 
-		/* Now shut down communication with child, we're done. */
-		shutdown(ipc_sockets[0], SHUT_RDWR);
-		close(ipc_sockets[0]);
-		lxc_proc_put_context_info(init_ctx);
-
 		/* We're done, the child process should now execute whatever it
 		 * is that the user requested. The parent can now track it with
 		 * waitpid() or similar.
 		 */
 
 		*attached_process = attached_pid;
-		return 0;
+
+		/* Now shut down communication with child, we're done. */
+		shutdown(ipc_sockets[0], SHUT_RDWR);
+		close(ipc_sockets[0]);
+		ipc_sockets[0] = -1;
+
+		ret_parent = 0;
+		to_cleanup_pid = -1;
+		if (options->attach_flags & LXC_ATTACH_ALLOCATE_PTY) {
+			ret = lxc_mainloop(&descr, -1);
+			if (ret < 0) {
+				ret_parent = -1;
+				to_cleanup_pid = attached_pid;
+			}
+		}
+
+	close_mainloop:
+		if (options->attach_flags & LXC_ATTACH_ALLOCATE_PTY)
+			lxc_mainloop_close(&descr);
 
 	on_error:
-		/* First shut down the socket, then wait for the pid, otherwise
-		 * the pid we're waiting for may never exit.
-		 */
+		if (ipc_sockets[0] >= 0) {
 			shutdown(ipc_sockets[0], SHUT_RDWR);
 			close(ipc_sockets[0]);
-		if (to_cleanup_pid)
+		}
+
+		if (to_cleanup_pid > 0)
 			(void)wait_for_pid(to_cleanup_pid);
+
+		if (options->attach_flags & LXC_ATTACH_ALLOCATE_PTY) {
+			lxc_console_delete(&pty);
+			lxc_pty_conf_free(&pty);
+		}
 		lxc_proc_put_context_info(init_ctx);
-		return -1;
+		return ret_parent;
 	}
 
-	/* First subprocess begins here, we close the socket that is for the
-	 * initial thread.
-	 */
+	/* close unneeded file descriptors */
 	close(ipc_sockets[0]);
+	ipc_sockets[0] = -EBADF;
+	if (options->attach_flags & LXC_ATTACH_ALLOCATE_PTY) {
+		lxc_attach_pty_close_master(&pty);
+		lxc_attach_pty_close_peer(&pty);
+		lxc_attach_pty_close_log(&pty);
+	}
 
 	/* Wait for the parent to have setup cgroups. */
-	expected = 0;
 	ret = lxc_read_nointr(ipc_sockets[1], &status, sizeof(status));
-	if (ret != sizeof(status) || status != expected) {
+	if (ret != sizeof(status)) {
 		shutdown(ipc_sockets[1], SHUT_RDWR);
 		lxc_proc_put_context_info(init_ctx);
 		rexit(-1);
@@ -1358,6 +1505,7 @@ int lxc_attach(const char *name, const char *lxcpath,
 	payload.ipc_socket = ipc_sockets[1];
 	payload.options = options;
 	payload.init_ctx = init_ctx;
+	payload.pty_fd = pty.slave;
 	payload.exec_function = exec_function;
 	payload.exec_payload = exec_payload;
 
@@ -1375,6 +1523,8 @@ int lxc_attach(const char *name, const char *lxcpath,
 			ERROR("Failed to exec");
 		_exit(EXIT_FAILURE);
 	}
+	if (options->attach_flags & LXC_ATTACH_ALLOCATE_PTY)
+		lxc_attach_pty_close_slave(&pty);
 
 	/* Tell grandparent the pid of the pid of the newly created child. */
 	ret = lxc_write_nointr(ipc_sockets[1], &pid, sizeof(pid));

src/lxc/attach_options.h

@@ -51,6 +51,7 @@ enum {
 	LXC_ATTACH_LSM_NOW               = 0x00020000, /*!< FIXME: unknown */
 	/* Set PR_SET_NO_NEW_PRIVS to block execve() gainable privileges. */
 	LXC_ATTACH_NO_NEW_PRIVS		 = 0x00040000, /*!< PR_SET_NO_NEW_PRIVS */
+	LXC_ATTACH_ALLOCATE_PTY          = 0x00080000, /*!< Allocate new pty for attached process. */
 
 	/* We have 16 bits for things that are on by default and 16 bits that
 	 * are off by default, that should be sufficient to keep binary
@@ -131,6 +132,9 @@ typedef struct lxc_attach_options_t {
 	int stdout_fd; /*!< stdout file descriptor */
 	int stderr_fd; /*!< stderr file descriptor */
 	/**@}*/
+
+	/*! File descriptor to log output. */
+	int log_fd;
 } lxc_attach_options_t;
 
 /*! Default attach options to use */
@@ -145,7 +149,10 @@ typedef struct lxc_attach_options_t {
 		/* .env_policy = */     LXC_ATTACH_KEEP_ENV,                   \
 		/* .extra_env_vars = */ NULL,                                  \
 		/* .extra_keep_env = */ NULL,                                  \
-		/* .stdin_fd = */       0, 1, 2                                \
+		/* .stdin_fd = */       0,                                     \
+		/* .stdout_fd = */      1,                                     \
+		/* .stderr_fd = */      2,                                     \
+		/* .log_fd    = */      -EBADF,                                \
 	}
 
 /*!

src/lxc/conf.c

@@ -73,6 +73,7 @@
 #include "cgroup.h"
 #include "conf.h"
 #include "confile_utils.h"
+#include "console.h"
 #include "error.h"
 #include "log.h"
 #include "lxclock.h"
@@ -3091,24 +3092,6 @@ int chown_mapped_root(const char *path, struct lxc_conf *conf)
 	return ret;
 }
 
-int lxc_ttys_shift_ids(struct lxc_conf *c)
-{
-	if (lxc_list_empty(&c->id_map))
-		return 0;
-
-	if (!strcmp(c->console.name, ""))
-		return 0;
-
-	if (chown_mapped_root(c->console.name, c) < 0) {
-		ERROR("failed to chown console \"%s\"", c->console.name);
-		return -1;
-	}
-
-	TRACE("chowned console \"%s\"", c->console.name);
-
-	return 0;
-}
-
 /* NOTE: Must not be called from inside the container namespace! */
 int lxc_create_tmp_proc_mount(struct lxc_conf *conf)
 {
@@ -3720,11 +3703,7 @@ void lxc_conf_free(struct lxc_conf *conf)
 		return;
 	if (current_config == conf)
 		current_config = NULL;
-	free(conf->console.buffer_log_file);
-	free(conf->console.log_path);
-	free(conf->console.path);
-	if (conf->console.buffer_size > 0 && conf->console.ringbuf.addr)
-		lxc_ringbuf_release(&conf->console.ringbuf);
+	lxc_pty_conf_free(&conf->console);
 	free(conf->rootfs.mount);
 	free(conf->rootfs.bdev_type);
 	free(conf->rootfs.options);

src/lxc/conf.h

@@ -174,7 +174,7 @@ struct lxc_console {
 	char name[MAXPATHLEN];
 	struct termios *tios;
 	struct lxc_tty_state *tty_state;
-
+	struct /* lxc_console_ringbuf */ {
 		/* size of the ringbuffer */
 		uint64_t buffer_size;
 
@@ -186,6 +186,7 @@ struct lxc_console {
 
 		/* the in-memory ringbuffer */
 		struct lxc_ringbuf ringbuf;
+	};
 };
 
 /*
@@ -451,7 +452,6 @@ extern int find_unmapped_nsid(struct lxc_conf *conf, enum idtype idtype);
 extern int mapped_hostid(unsigned id, struct lxc_conf *conf,
 			 enum idtype idtype);
 extern int chown_mapped_root(const char *path, struct lxc_conf *conf);
-extern int lxc_ttys_shift_ids(struct lxc_conf *c);
 extern int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data,
 			 const char *fn_name);
 extern int userns_exec_full(struct lxc_conf *conf, int (*fn)(void *),

src/lxc/console.c

@@ -168,7 +168,7 @@ struct lxc_tty_state *lxc_console_signal_init(int srcfd, int dstfd)
 		goto on_error;
 	}
 
-	ts->sigfd = signalfd(-1, &mask, 0);
+	ts->sigfd = signalfd(-1, &mask, SFD_CLOEXEC);
 	if (ts->sigfd < 0) {
 		WARN("Failed to create signal fd");
 		sigprocmask(SIG_SETMASK, &ts->oldmask, NULL);
@@ -285,31 +285,25 @@ static int lxc_console_mainloop_add_peer(struct lxc_console *console)
 	return 0;
 }
 
-extern int lxc_console_mainloop_add(struct lxc_epoll_descr *descr,
-				    struct lxc_conf *conf)
+int lxc_console_mainloop_add(struct lxc_epoll_descr *descr,
+			     struct lxc_console *console)
 {
 	int ret;
-	struct lxc_console *console = &conf->console;
-
-	if (!conf->rootfs.path) {
-		INFO("no rootfs, no console.");
-		return 0;
-	}
 
 	if (console->master < 0) {
 		INFO("no console");
 		return 0;
 	}
 
-	if (lxc_mainloop_add_handler(descr, console->master,
-				     lxc_console_cb_con, console)) {
-		ERROR("failed to add to mainloop console handler for '%d'",
-		      console->master);
+	ret = lxc_mainloop_add_handler(descr, console->master,
+				       lxc_console_cb_con, console);
+	if (ret < 0) {
+		ERROR("Failed to add handler for %d to mainloop", console->master);
 		return -1;
 	}
 
-	/* we cache the descr so that we can add an fd to it when someone
-	 * does attach to it in lxc_console_allocate()
+	/* We cache the descr so that we can add an fd to it when someone
+	 * does attach to it in lxc_console_allocate().
 	 */
 	console->descr = descr;
 	ret = lxc_console_mainloop_add_peer(console);
@@ -709,24 +703,13 @@ int lxc_console_create_log_file(struct lxc_console *console)
 	return 0;
 }
 
-int lxc_console_create(struct lxc_conf *conf)
+int lxc_pty_create(struct lxc_console *console)
 {
 	int ret, saved_errno;
-	struct lxc_console *console = &conf->console;
-
-	if (!conf->rootfs.path) {
-		INFO("Container does not have a rootfs. The console will be "
-		     "shared with the host");
-		return 0;
-	}
-
-	if (console->path && !strcmp(console->path, "none")) {
-		INFO("No console was requested");
-		return 0;
-	}
 
 	process_lock();
-	ret = openpty(&console->master, &console->slave, console->name, NULL, NULL);
+	ret = openpty(&console->master, &console->slave, console->name, NULL,
+		      NULL);
 	saved_errno = errno;
 	process_unlock();
 	if (ret < 0) {
@@ -752,6 +735,33 @@ int lxc_console_create(struct lxc_conf *conf)
 		goto err;
 	}
 
+	return 0;
+
+err:
+	lxc_console_delete(console);
+	return -ENODEV;
+}
+
+int lxc_console_create(struct lxc_conf *conf)
+{
+	int ret;
+	struct lxc_console *console = &conf->console;
+
+	if (!conf->rootfs.path) {
+		INFO("Container does not have a rootfs. The console will be "
+		     "shared with the host");
+		return 0;
+	}
+
+	if (console->path && !strcmp(console->path, "none")) {
+		INFO("No console was requested");
+		return 0;
+	}
+
+	ret = lxc_pty_create(console);
+	if (ret < 0)
+		return -1;
+
 	/* create console log file */
 	ret = lxc_console_create_log_file(console);
 	if (ret < 0)
@@ -969,3 +979,84 @@ close_fds:
 
 	return ret;
 }
+
+int lxc_make_controlling_pty(int fd)
+{
+	int ret;
+
+	setsid();
+
+	ret = ioctl(fd, TIOCSCTTY, (char *)NULL);
+	if (ret < 0)
+		return -1;
+
+	return 0;
+}
+
+int lxc_login_pty(int fd)
+{
+	int ret;
+
+	ret = lxc_make_controlling_pty(fd);
+	if (ret < 0)
+		return -1;
+
+	ret = lxc_console_set_stdfds(fd);
+	if (ret < 0)
+		return -1;
+
+	if (fd > STDERR_FILENO)
+		close(fd);
+
+	return 0;
+}
+
+void lxc_pty_info_init(struct lxc_pty_info *pty)
+{
+	pty->name[0] = '\0';
+	pty->master = -EBADF;
+	pty->slave = -EBADF;
+	pty->busy = -1;
+}
+
+void lxc_pty_init(struct lxc_console *pty)
+{
+	memset(pty, 0, sizeof(*pty));
+	pty->slave = -EBADF;
+	pty->master = -EBADF;
+	pty->peer = -EBADF;
+	pty->log_fd = -EBADF;
+	pty->buffer_log_file_fd = -EBADF;
+	lxc_pty_info_init(&pty->peerpty);
+}
+
+void lxc_pty_conf_free(struct lxc_console *console)
+{
+	free(console->buffer_log_file);
+	free(console->log_path);
+	free(console->path);
+	if (console->buffer_size > 0 && console->ringbuf.addr)
+		lxc_ringbuf_release(&console->ringbuf);
+}
+
+int lxc_pty_map_ids(struct lxc_conf *c, struct lxc_console *pty)
+{
+	int ret;
+
+	if (lxc_list_empty(&c->id_map))
+		return 0;
+
+	ret = strcmp(pty->name, "");
+	if (ret == 0)
+		return 0;
+
+	ret = chown_mapped_root(pty->name, c);
+	if (ret < 0) {
+		ERROR("Failed to chown \"%s\"", pty->name);
+		return -1;
+	}
+
+	TRACE("Chowned \"%s\"", pty->name);
+
+	return 0;
+}

src/lxc/console.h

@@ -82,6 +82,12 @@ extern int  lxc_console_allocate(struct lxc_conf *conf, int sockfd, int *ttynum)
  * automatically chowned to the uid/gid of the unprivileged user. For this
  * ttys_shift_ids() can be called.)
  */
+extern int lxc_pty_create(struct lxc_console *console);
+
+/**
+ * lxc_console_create: Create a new pty.
+ * - In addition to lxc_pty_create() also sets up all pty logs.
+ */
 extern int  lxc_console_create(struct lxc_conf *);
 
 /*
@@ -108,7 +114,7 @@ extern void lxc_console_free(struct lxc_conf *conf, int fd);
 /*
  * Register pty event handlers in an open mainloop
  */
-extern int  lxc_console_mainloop_add(struct lxc_epoll_descr *, struct lxc_conf *);
+extern int  lxc_console_mainloop_add(struct lxc_epoll_descr *, struct lxc_console *);
 
 /*
  * Handle SIGWINCH events on the allocated ptys.
@@ -226,4 +232,11 @@ extern int lxc_console_create_log_file(struct lxc_console *console);
 extern int lxc_console_cb_con(int fd, uint32_t events, void *data,
 			      struct lxc_epoll_descr *descr);
 
+extern int lxc_make_controlling_pty(int fd);
+extern int lxc_login_pty(int fd);
+extern void lxc_pty_conf_free(struct lxc_console *console);
+extern void lxc_pty_info_init(struct lxc_pty_info *pty);
+extern void lxc_pty_init(struct lxc_console *pty);
+extern int lxc_pty_map_ids(struct lxc_conf *c, struct lxc_console *pty);
+
 #endif

src/lxc/start.c

@@ -266,35 +266,36 @@ restart:
 
 static int setup_signal_fd(sigset_t *oldmask)
 {
+	int ret, sig;
 	sigset_t mask;
-	int fd;
+	int signals[] = {SIGBUS, SIGILL, SIGSEGV, SIGWINCH};
 
 	/* Block everything except serious error signals. */
-	if (sigfillset(&mask) ||
-	    sigdelset(&mask, SIGILL) ||
-	    sigdelset(&mask, SIGSEGV) ||
-	    sigdelset(&mask, SIGBUS) ||
-	    sigdelset(&mask, SIGWINCH) ||
-	    sigprocmask(SIG_BLOCK, &mask, oldmask)) {
-		SYSERROR("Failed to set signal mask.");
-		return -1;
+	ret = sigfillset(&mask);
+	if (ret < 0)
+		return -EBADF;
+
+	for (sig = 0; sig < (sizeof(signals) / sizeof(signals[0])); sig++) {
+		ret = sigdelset(&mask, signals[sig]);
+		if (ret < 0)
+			return -EBADF;
 	}
 
-	fd = signalfd(-1, &mask, 0);
-	if (fd < 0) {
-		SYSERROR("Failed to create signal file descriptor.");
-		return -1;
+	ret = sigprocmask(SIG_BLOCK, &mask, oldmask);
+	if (ret < 0) {
+		SYSERROR("Failed to set signal mask");
+		return -EBADF;
 	}
 
-	if (fcntl(fd, F_SETFD, FD_CLOEXEC)) {
-		SYSERROR("Failed to set FD_CLOEXEC on the signal file descriptor: %d.", fd);
-		close(fd);
-		return -1;
+	ret = signalfd(-1, &mask, SFD_CLOEXEC);
+	if (ret < 0) {
+		SYSERROR("Failed to create signal file descriptor");
+		return -EBADF;
 	}
 
-	DEBUG("Set SIGCHLD handler with file descriptor: %d.", fd);
+	TRACE("Created signal file descriptor %d", ret);
 
-	return fd;
+	return ret;
 }
 
 static int signal_handler(int fd, uint32_t events, void *data,
@@ -468,6 +469,7 @@ int lxc_set_state(const char *name, struct lxc_handler *handler,
 int lxc_poll(const char *name, struct lxc_handler *handler)
 {
 	int ret;
+	bool has_console = (handler->conf->rootfs.path != NULL);
 	struct lxc_epoll_descr descr, descr_console;
 
 	ret = lxc_mainloop_open(&descr);
@@ -476,11 +478,13 @@ int lxc_poll(const char *name, struct lxc_handler *handler)
 		goto out_sigfd;
 	}
 
+	if (has_console) {
 		ret = lxc_mainloop_open(&descr_console);
 		if (ret < 0) {
 			ERROR("Failed to create console mainloop");
 			goto out_mainloop;
 		}
+	}
 
 	ret = lxc_mainloop_add_handler(&descr, handler->sigfd, signal_handler, handler);
 	if (ret < 0) {
@@ -488,17 +492,21 @@ int lxc_poll(const char *name, struct lxc_handler *handler)
 		goto out_mainloop_console;
 	}
 
-	ret = lxc_console_mainloop_add(&descr, handler->conf);
+	if (has_console) {
+		struct lxc_console *console = &handler->conf->console;
+
+		ret = lxc_console_mainloop_add(&descr, console);
 		if (ret < 0) {
 			ERROR("Failed to add console handlers to mainloop");
 			goto out_mainloop_console;
 		}
 
-	ret = lxc_console_mainloop_add(&descr_console, handler->conf);
+		ret = lxc_console_mainloop_add(&descr_console, console);
 		if (ret < 0) {
 			ERROR("Failed to add console handlers to console mainloop");
 			goto out_mainloop_console;
 		}
+	}
 
 	ret = lxc_cmd_mainloop_add(name, &descr, handler);
 	if (ret < 0) {
@@ -514,15 +522,19 @@ int lxc_poll(const char *name, struct lxc_handler *handler)
 	if (ret < 0 || !handler->init_died)
 		goto out_mainloop;
 
+	if (has_console)
 		ret = lxc_mainloop(&descr_console, 0);
 
+
 out_mainloop:
 	lxc_mainloop_close(&descr);
 	TRACE("Closed mainloop");
 
 out_mainloop_console:
+	if (has_console) {
 		lxc_mainloop_close(&descr_console);
 		TRACE("Closed console mainloop");
+	}
 
 out_sigfd:
 	close(handler->sigfd);
@@ -739,13 +751,15 @@ int lxc_init(const char *name, struct lxc_handler *handler)
 	TRACE("set up signal fd");
 
 	/* Do this after setting up signals since it might unblock SIGWINCH. */
-	if (lxc_console_create(conf)) {
-		ERROR("Failed to create console for container \"%s\".", name);
+	ret = lxc_console_create(conf);
+	if (ret < 0) {
+		ERROR("Failed to create console");
 		goto out_restore_sigmask;
 	}
-	TRACE("created console");
+	TRACE("Created console");
 
-	if (lxc_ttys_shift_ids(conf) < 0) {
+	ret = lxc_pty_map_ids(conf, &conf->console);
+	if (ret < 0) {
 		ERROR("Failed to shift tty into container.");
 		goto out_restore_sigmask;
 	}

src/lxc/tools/lxc_attach.c

@@ -27,11 +27,12 @@
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <termios.h>
+#include <unistd.h>
 #include <sys/ioctl.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
-#include <termios.h>
-#include <unistd.h>
 
 #include <lxc/lxccontainer.h>
 
@@ -46,12 +47,6 @@
 #include "mainloop.h"
 #include "utils.h"
 
-#if HAVE_PTY_H
-#include <pty.h>
-#else
-#include <../include/openpty.h>
-#endif
-
 static const struct option my_longopts[] = {
 	{"elevated-privileges", optional_argument, 0, 'e'},
 	{"arch", required_argument, 0, 'a'},
@@ -241,154 +236,29 @@ Options :\n\
 	.checker  = NULL,
 };
 
-struct wrapargs {
-	lxc_attach_options_t *options;
-	lxc_attach_command_t *command;
-	struct lxc_console *console;
-	int ptyfd;
-};
-
-/* Minimalistic login_tty() implementation. */
-static int login_pty(int fd)
+static bool stdfd_is_pty(void)
 {
-	setsid();
-	if (ioctl(fd, TIOCSCTTY, NULL) < 0)
-		return -1;
-	if (lxc_console_set_stdfds(fd) < 0)
-		return -1;
-	if (fd > STDERR_FILENO)
-		close(fd);
-	return 0;
-}
-
-static int get_pty_on_host_callback(void *p)
-{
-	struct wrapargs *wrap = p;
-
-	close(wrap->console->master);
-	if (login_pty(wrap->console->slave) < 0)
-		return -1;
+	if (isatty(STDIN_FILENO))
+		return true;
+	if (isatty(STDOUT_FILENO))
+		return true;
+	if (isatty(STDERR_FILENO))
+		return true;
 
-	if (wrap->command->program)
-		lxc_attach_run_command(wrap->command);
-	else
-		lxc_attach_run_shell(NULL);
-	return -1;
+	return false;
 }
 
-static int get_pty_on_host(struct lxc_container *c, struct wrapargs *wrap, int *pid)
+int lxc_attach_create_log_file(const char *log_file)
 {
-	struct lxc_epoll_descr descr;
-	struct lxc_conf *conf;
-	struct lxc_tty_state *ts;
-	int ret = -1;
-	struct wrapargs *args = wrap;
-
-	if (!isatty(args->ptyfd)) {
-		fprintf(stderr, "Standard file descriptor does not refer to a pty\n");
-		return -1;
-	}
-
-	if (c->lxc_conf) {
-		conf = c->lxc_conf;
-	} else {
-		/* If the container is not defined and the user didn't specify a
-		 * config file to load we will simply init a dummy config here.
-		 */
-		conf = lxc_conf_init();
-		if (!conf) {
-			fprintf(stderr, "Failed to allocate dummy config file for the container\n");
-			return -1;
-		}
-
-		/* We also need a dummy rootfs path otherwise
-		 * lxc_console_create() will not let us create a console. Note,
-		 * I don't want this change to make it into
-		 * lxc_console_create()'s since this function will only be
-		 * responsible for proper /dev/{console,tty<n>} devices.
-		 * lxc-attach is just abusing it to also handle the pty case
-		 * because it is very similar. However, with LXC 3.0 lxc-attach
-		 * will need to move away from using lxc_console_create() since
-		 * this is actually an internal symbol and we only want the
-		 * tools to use the API with LXC 3.0.
-		 */
-		conf->rootfs.path = strdup("dummy");
-		if (!conf->rootfs.path)
-			return -1;
-	}
-	free(conf->console.log_path);
-	if (my_args.console_log)
-		conf->console.log_path = strdup(my_args.console_log);
-	else
-		conf->console.log_path = NULL;
+	int fd;
 
-	/* In the case of lxc-attach our peer pty will always be the current
-	 * controlling terminal. We clear whatever was set by the user for
-	 * lxc.console.path here and set it NULL. lxc_console_peer_default()
-	 * will then try to open /dev/tty. If the process doesn't have a
-	 * controlling terminal we should still proceed.
-	 */
-	free(conf->console.path);
-	conf->console.path = NULL;
-
-	/* Create pty on the host. */
-	if (lxc_console_create(conf) < 0)
+	fd = open(log_file, O_CLOEXEC | O_RDWR | O_CREAT | O_APPEND, 0600);
+	if (fd < 0) {
+		fprintf(stderr, "Failed to open log file \"%s\"\n", log_file);
 		return -1;
-	ts = conf->console.tty_state;
-	conf->console.descr = &descr;
-
-	/* Shift ttys to container. */
-	if (lxc_ttys_shift_ids(conf) < 0) {
-		fprintf(stderr, "Failed to shift tty into container\n");
-		goto err1;
 	}
 
-	/* Send wrapper function on its way. */
-	wrap->console = &conf->console;
-	if (c->attach(c, get_pty_on_host_callback, wrap, wrap->options, pid) < 0)
-		goto err1;
-	close(conf->console.slave); /* Close slave side. */
-	conf->console.slave = -1;
-
-	ret = lxc_mainloop_open(&descr);
-	if (ret) {
-		fprintf(stderr, "failed to create mainloop\n");
-		goto err2;
-	}
-
-	if (lxc_console_mainloop_add(&descr, conf) < 0) {
-		fprintf(stderr, "Failed to add handlers to lxc mainloop.\n");
-		goto err3;
-	}
-
-	ret = lxc_mainloop(&descr, -1);
-	if (ret) {
-		fprintf(stderr, "mainloop returned an error\n");
-		goto err3;
-	}
-	ret = 0;
-
-err3:
-	lxc_mainloop_close(&descr);
-err2:
-	if (ts && ts->sigfd != -1)
-		lxc_console_signal_fini(ts);
-err1:
-	lxc_console_delete(&conf->console);
-
-	return ret;
-}
-
-static int stdfd_is_pty(void)
-{
-	if (isatty(STDIN_FILENO))
-		return STDIN_FILENO;
-	if (isatty(STDOUT_FILENO))
-		return STDOUT_FILENO;
-	if (isatty(STDERR_FILENO))
-		return STDERR_FILENO;
-
-	return -1;
+	return fd;
 }
 
 int main(int argc, char *argv[])
@@ -462,6 +332,8 @@ int main(int argc, char *argv[])
 		attach_options.attach_flags |= LXC_ATTACH_REMOUNT_PROC_SYS;
 	if (elevated_privileges)
 		attach_options.attach_flags &= ~(elevated_privileges);
+	if (stdfd_is_pty())
+		attach_options.attach_flags |= LXC_ATTACH_ALLOCATE_PTY;
 	attach_options.namespaces = namespace_flags;
 	attach_options.personality = new_personality;
 	attach_options.env_policy = env_policy;
@@ -473,28 +345,16 @@ int main(int argc, char *argv[])
 		command.argv = (char**)my_args.argv;
 	}
 
-	struct wrapargs wrap = (struct wrapargs){
-		.command = &command,
-		.options = &attach_options
-	};
-
-	wrap.ptyfd = stdfd_is_pty();
-	if (wrap.ptyfd >= 0) {
-		if ((!isatty(STDOUT_FILENO) || !isatty(STDERR_FILENO)) && my_args.console_log) {
-			fprintf(stderr, "-L/--pty-log can only be used when stdout and stderr refer to a pty.\n");
-			goto out;
-		}
-		ret = get_pty_on_host(c, &wrap, &pid);
-	} else {
 	if (my_args.console_log) {
-			fprintf(stderr, "-L/--pty-log can only be used when stdout and stderr refer to a pty.\n");
+		attach_options.log_fd = lxc_attach_create_log_file(my_args.console_log);
+		if (attach_options.log_fd < 0)
 			goto out;
 	}
+
 	if (command.program)
 		ret = c->attach(c, lxc_attach_run_command, &command, &attach_options, &pid);
 	else
 		ret = c->attach(c, lxc_attach_run_shell, NULL, &attach_options, &pid);
-	}
 
 	if (ret < 0)
 		goto out;

src/lxc/tools/lxc_start.c

@@ -278,6 +278,14 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	if (my_args.console)
+		if (!c->set_config_item(c, "lxc.console.path", my_args.console))
+			goto out;
+
+	if (my_args.console_log)
+		if (!c->set_config_item(c, "lxc.console.logfile", my_args.console_log))
+			goto out;
+
 	if (!lxc_setup_shared_ns(&my_args, c))
 		goto out;
 

src/lxc/utils.c

@@ -2396,7 +2396,7 @@ int parse_byte_size_string(const char *s, int64_t *converted)
 	int64_t mltpl, overflow;
 	char *end;
 	char dup[LXC_NUMSTRLEN64 + 2];
-	char suffix[3];
+	char suffix[3] = {0};
 
 	if (!s || !strcmp(s, ""))
 		return -EINVAL;
@@ -2412,10 +2412,10 @@ int parse_byte_size_string(const char *s, int64_t *converted)
 	else
 		return -EINVAL;
 
-	if ((end - 2) == dup && !isdigit(*(end - 2)))
+	if (suffix_len > 0 && (end - 2) == dup && !isdigit(*(end - 2)))
 		return -EINVAL;
 
-	if (isalpha(*(end - 2))) {
+	if (suffix_len > 0 && isalpha(*(end - 2))) {
 		if (suffix_len == 1)
 			suffix_len++;
 		else

src/tests/attach.c

@@ -19,16 +19,18 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-#include <lxc/lxccontainer.h>
-#include "lxc/utils.h"
-#include "lxc/lsm/lsm.h"
-
-#include <sys/types.h>
+#include <errno.h>
 #include <string.h>
 #include <unistd.h>
 #include <sys/stat.h>
 #include <sys/syscall.h>
-#include <errno.h>
+#include <sys/types.h>
+
+#include "lxctest.h"
+#include "utils.h"
+#include "lsm/lsm.h"
+
+#include <lxc/lxccontainer.h>
 
 #define TSTNAME    "lxc-attach-test"
 #define TSTOUT(fmt, ...) do { \
@@ -392,19 +394,60 @@ err1:
 
 int main(int argc, char *argv[])
 {
-	int ret;
+	int i, ret;
+	struct lxc_log log;
+	char template[sizeof(P_tmpdir"/attach_XXXXXX")];
+	int fret = EXIT_FAILURE;
+
+	strcpy(template, P_tmpdir"/attach_XXXXXX");
+	i = lxc_make_tmpfile(template, false);
+	if (i < 0) {
+		lxc_error("Failed to create temporary log file for container %s\n", TSTNAME);
+		exit(EXIT_FAILURE);
+	} else {
+		lxc_debug("Using \"%s\" as temporary log file for container %s\n", template, TSTNAME);
+		close(i);
+	}
+
+	log.name = TSTNAME;
+	log.file = template;
+	log.level = "TRACE";
+	log.prefix = "attach";
+	log.quiet = false;
+	log.lxcpath = NULL;
+	if (lxc_log_init(&log))
+		goto on_error;
 
 	test_lsm_detect();
 	ret = test_attach(NULL, TSTNAME, "busybox");
 	if (ret < 0)
-		return EXIT_FAILURE;
+		goto on_error;
 
 	TSTOUT("\n");
 	ret = test_attach(LXCPATH "/alternate-path-test", TSTNAME, "busybox");
 	if (ret < 0)
-		return EXIT_FAILURE;
+		goto on_error;
 
-	(void)rmdir(LXCPATH "/alternate-path-test");
 	TSTOUT("All tests passed\n");
-	return EXIT_SUCCESS;
+	fret = EXIT_SUCCESS;
+
+on_error:
+	if (fret != EXIT_SUCCESS) {
+		int fd;
+
+		fd = open(template, O_RDONLY);
+		if (fd >= 0) {
+			char buf[4096];
+			ssize_t buflen;
+			while ((buflen = read(fd, buf, 1024)) > 0) {
+				buflen = write(STDERR_FILENO, buf, buflen);
+				if (buflen <= 0)
+					break;
+			}
+			close(fd);
+		}
+	}
+	(void)rmdir(LXCPATH "/alternate-path-test");
+	(void)unlink(template);
+	exit(fret);
 }

src/tests/lxc-test-lxc-attach

@@ -190,17 +190,6 @@ fi
 
 rm -f $out $err
 
-if [ $allocate_pty = "pty" ]; then
-	# Test whether logging pty output to a file works.
-	trap "rm -f /tmp/ptylog" EXIT INT QUIT PIPE
-	lxc-attach -n busy -L /tmp/ptylog -- hostname || FAIL "to allocate or setup pty"
-	if [ ! -s /tmp/ptylog ]; then
-		FAIL "lxc-attach -n busy -L /tmp/ptylog -- hostname"
-	fi
-
-	rm -f /tmp/ptylog
-fi
-
 lxc-destroy -n busy -f
 
 exit 0

src/tests/lxc-test-utils.c

@@ -180,6 +180,7 @@ void test_detect_ramfs_rootfs(void)
 	}
 	fclose(fp1);
 	fp1 = NULL;
+	fd1 = -1;
 
 	/* Test if it correctly fails to detect when no - rootfs rootfs */
 	for (i = 0; i < sizeof(mountinfo) / sizeof(mountinfo[0]); i++) {
@@ -192,6 +193,7 @@ void test_detect_ramfs_rootfs(void)
 	}
 	fclose(fp2);
 	fp2 = NULL;
+	fd2 = -1;
 
 	if (mount(tmpf1, "/proc/self/mountinfo", NULL, MS_BIND, 0) < 0) {
 		lxc_error("%s\n", "Could not overmount \"/proc/self/mountinfo\".");
@@ -386,80 +388,108 @@ void test_parse_byte_size_string(void)
 	int64_t n;
 
 	ret = parse_byte_size_string("0", &n);
-	if (ret < 0)
+	if (ret < 0) {
+		lxc_error("%s\n", "Failed to parse \"0\"");
 		exit(EXIT_FAILURE);
-	if (n != 0)
+	}
+	if (n != 0) {
+		lxc_error("%s\n", "Failed to parse \"0\"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("1", &n);
-	if (ret < 0)
+	if (ret < 0) {
+		lxc_error("%s\n", "Failed to parse \"1\"");
 		exit(EXIT_FAILURE);
-	if (n != 1)
+	}
+	if (n != 1) {
+		lxc_error("%s\n", "Failed to parse \"1\"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("1 ", &n);
-	if (ret == 0)
+	if (ret == 0) {
+		lxc_error("%s\n", "Failed to parse \"1 \"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("1B", &n);
-	if (ret < 0)
+	if (ret < 0) {
+		lxc_error("%s\n", "Failed to parse \"1B\"");
 		exit(EXIT_FAILURE);
-	if (n != 1)
+	}
+	if (n != 1) {
+		lxc_error("%s\n", "Failed to parse \"1B\"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("1kB", &n);
-	if (ret < 0)
+	if (ret < 0) {
+		lxc_error("%s\n", "Failed to parse \"1kB\"");
 		exit(EXIT_FAILURE);
-	if (n != 1024)
+	}
+	if (n != 1024) {
+		lxc_error("%s\n", "Failed to parse \"1kB\"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("1MB", &n);
-	if (ret < 0)
-		exit(EXIT_FAILURE);
-	if (n != 1048576)
+	if (ret < 0) {
+		lxc_error("%s\n", "Failed to parse \"1MB\"");
 		exit(EXIT_FAILURE);
-
-	ret = parse_byte_size_string("1GB", &n);
-	if (ret < 0)
-		exit(EXIT_FAILURE);
-	if (n != 1073741824)
+	}
+	if (n != 1048576) {
+		lxc_error("%s\n", "Failed to parse \"1MB\"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("1TB", &n);
-	if (ret == 0)
+	if (ret == 0) {
+		lxc_error("%s\n", "Failed to parse \"1TB\"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("1 B", &n);
-	if (ret < 0)
+	if (ret < 0) {
+		lxc_error("%s\n", "Failed to parse \"1 B\"");
 		exit(EXIT_FAILURE);
-	if (n != 1)
+	}
+	if (n != 1) {
+		lxc_error("%s\n", "Failed to parse \"1 B\"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("1 kB", &n);
-	if (ret < 0)
+	if (ret < 0) {
+		lxc_error("%s\n", "Failed to parse \"1 kB\"");
 		exit(EXIT_FAILURE);
-	if (n != 1024)
+	}
+	if (n != 1024) {
+		lxc_error("%s\n", "Failed to parse \"1 kB\"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("1 MB", &n);
-	if (ret < 0)
-		exit(EXIT_FAILURE);
-	if (n != 1048576)
+	if (ret < 0) {
+		lxc_error("%s\n", "Failed to parse \"1 MB\"");
 		exit(EXIT_FAILURE);
-
-	ret = parse_byte_size_string("1 GB", &n);
-	if (ret < 0)
-		exit(EXIT_FAILURE);
-	if (n != 1073741824)
+	}
+	if (n != 1048576) {
+		lxc_error("%s\n", "Failed to parse \"1 MB\"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("1 TB", &n);
-	if (ret == 0)
+	if (ret == 0) {
+		lxc_error("%s\n", "Failed to parse \"1 TB\"");
 		exit(EXIT_FAILURE);
+	}
 
 	ret = parse_byte_size_string("asdf", &n);
-	if (ret == 0)
+	if (ret == 0) {
+		lxc_error("%s\n", "Failed to parse \"asdf\"");
 		exit(EXIT_FAILURE);
+	}
 }
 
 void test_lxc_config_net_hwaddr(void)