Merge pull request #3637 from brauner/2021-01-27/fixes

src/lxc/attach.c

@@ -109,8 +109,7 @@ 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);
-	ctx->lsm_label = NULL;
+	free_disarm(ctx->lsm_label);
 
 	if (ctx->container) {
 		lxc_container_put(ctx->container);
@@ -645,7 +644,7 @@ static void lxc_put_attach_clone_payload(struct attach_clone_payload *p)
 	}
 }
 
-static int attach_child_main(struct attach_clone_payload *payload)
+__noreturn static void do_attach(struct attach_clone_payload *payload)
 {
 	int lsm_fd, ret;
 	uid_t new_uid;
@@ -818,8 +817,7 @@ static int attach_child_main(struct attach_clone_payload *payload)
 			goto on_error;
 	}
 
-	close(payload->ipc_socket);
-	payload->ipc_socket = -EBADF;
+	close_prot_errno_disarm(payload->ipc_socket);
 	lxc_proc_put_context_info(init_ctx);
 	payload->init_ctx = NULL;
 
@@ -833,13 +831,16 @@ static int attach_child_main(struct attach_clone_payload *payload)
 	 * may want to make sure the fds are closed, for example.
 	 */
 	if (options->stdin_fd >= 0 && options->stdin_fd != STDIN_FILENO)
-		(void)dup2(options->stdin_fd, STDIN_FILENO);
+		if (dup2(options->stdin_fd, STDIN_FILENO))
+			DEBUG("Failed to replace stdin with %d", options->stdin_fd);
 
 	if (options->stdout_fd >= 0 && options->stdout_fd != STDOUT_FILENO)
-		(void)dup2(options->stdout_fd, STDOUT_FILENO);
+		if (dup2(options->stdout_fd, STDOUT_FILENO))
+			DEBUG("Failed to replace stdout with %d", options->stdin_fd);
 
 	if (options->stderr_fd >= 0 && options->stderr_fd != STDERR_FILENO)
-		(void)dup2(options->stderr_fd, STDERR_FILENO);
+		if (dup2(options->stderr_fd, STDERR_FILENO))
+			DEBUG("Failed to replace stderr with %d", options->stdin_fd);
 
 	/* close the old fds */
 	if (options->stdin_fd > STDERR_FILENO)
@@ -893,6 +894,7 @@ static int attach_child_main(struct attach_clone_payload *payload)
 
 on_error:
 	lxc_put_attach_clone_payload(payload);
+	ERROR("Failed to attach to container");
 	_exit(EXIT_FAILURE);
 }
 
@@ -952,9 +954,10 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 	       void *exec_payload, lxc_attach_options_t *options,
 	       pid_t *attached_process)
 {
+	__do_free char *cwd = NULL;
 	int i, ret, status;
 	int ipc_sockets[2];
-	char *cwd, *new_cwd;
+	char *new_cwd;
 	signed long personality;
 	pid_t attached_pid, init_pid, pid;
 	struct lxc_proc_context_info *init_ctx;
@@ -962,6 +965,9 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 	struct lxc_conf *conf;
 	char *name, *lxcpath;
 	struct attach_clone_payload payload = {0};
+	int ret_parent = -1;
+	pid_t to_cleanup_pid;
+	struct lxc_epoll_descr descr = {0};
 
 	ret = access("/proc/self/ns", X_OK);
 	if (ret)
@@ -1030,7 +1036,6 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 		if (options->namespaces == -1) {
 			ERROR("Failed to automatically determine the "
 			      "namespaces which the container uses");
-			free(cwd);
 			lxc_proc_put_context_info(init_ctx);
 			return -1;
 		}
@@ -1080,7 +1085,6 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 		for (j = 0; j < i; j++)
 			close(init_ctx->ns_fd[j]);
 
-		free(cwd);
 		lxc_proc_put_context_info(init_ctx);
 		return -1;
 	}
@@ -1089,7 +1093,6 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 		ret = lxc_attach_terminal(name, lxcpath, conf, &terminal);
 		if (ret < 0) {
 			ERROR("Failed to setup new terminal");
-			free(cwd);
 			lxc_proc_put_context_info(init_ctx);
 			return -1;
 		}
@@ -1135,7 +1138,6 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 	ret = socketpair(PF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets);
 	if (ret < 0) {
 		SYSERROR("Could not set up required IPC mechanism for attaching");
-		free(cwd);
 		lxc_proc_put_context_info(init_ctx);
 		return -1;
 	}
@@ -1150,19 +1152,112 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 	pid = fork();
 	if (pid < 0) {
 		SYSERROR("Failed to create first subprocess");
-		free(cwd);
 		lxc_proc_put_context_info(init_ctx);
 		return -1;
 	}
 
-	if (pid) {
-		int ret_parent = -1;
-		pid_t to_cleanup_pid = pid;
-		struct lxc_epoll_descr descr = {0};
+	if (pid == 0) {
+		/* close unneeded file descriptors */
+		close_prot_errno_disarm(ipc_sockets[0]);
+
+		if (options->attach_flags & LXC_ATTACH_TERMINAL) {
+			lxc_attach_terminal_close_ptx(&terminal);
+			lxc_attach_terminal_close_peer(&terminal);
+			lxc_attach_terminal_close_log(&terminal);
+		}
+
+		/* Wait for the parent to have setup cgroups. */
+		ret = lxc_read_nointr(ipc_sockets[1], &status, sizeof(status));
+		if (ret != sizeof(status)) {
+			shutdown(ipc_sockets[1], SHUT_RDWR);
+			lxc_proc_put_context_info(init_ctx);
+			_exit(EXIT_FAILURE);
+		}
+
+		TRACE("Intermediate process starting to initialize");
+
+		/* Attach now, create another subprocess later, since pid namespaces
+		 * only really affect the children of the current process.
+		 */
+		ret = lxc_attach_to_ns(init_pid, init_ctx);
+		if (ret < 0) {
+			ERROR("Failed to enter namespaces");
+			shutdown(ipc_sockets[1], SHUT_RDWR);
+			lxc_proc_put_context_info(init_ctx);
+			_exit(EXIT_FAILURE);
+		}
+
+		/* close namespace file descriptors */
+		lxc_proc_close_ns_fd(init_ctx);
+
+		/* Attach succeeded, try to cwd. */
+		if (options->initial_cwd)
+			new_cwd = options->initial_cwd;
+		else
+			new_cwd = cwd;
+		if (new_cwd) {
+			ret = chdir(new_cwd);
+			if (ret < 0)
+				WARN("Could not change directory to \"%s\"", new_cwd);
+		}
+
+		/* Create attached process. */
+		payload.ipc_socket	= ipc_sockets[1];
+		payload.options		= options;
+		payload.init_ctx	= init_ctx;
+		payload.terminal_pts_fd = terminal.pty;
+		payload.exec_function	= exec_function;
+		payload.exec_payload	= exec_payload;
+
+		pid = lxc_raw_clone(CLONE_PARENT, NULL);
+		if (pid < 0) {
+			SYSERROR("Failed to clone attached process");
+			shutdown(ipc_sockets[1], SHUT_RDWR);
+			lxc_proc_put_context_info(init_ctx);
+			_exit(EXIT_FAILURE);
+		}
+
+		if (pid == 0) {
+			if (options->attach_flags & LXC_ATTACH_TERMINAL) {
+				ret = lxc_terminal_signal_sigmask_safe_blocked(&terminal);
+				if (ret < 0) {
+					SYSERROR("Failed to reset signal mask");
+					_exit(EXIT_FAILURE);
+				}
+			}
+
+			do_attach(&payload);
+		}
+
+		if (options->attach_flags & LXC_ATTACH_TERMINAL)
+			lxc_attach_terminal_close_pts(&terminal);
+
+		/* Tell grandparent the pid of the pid of the newly created child. */
+		ret = lxc_write_nointr(ipc_sockets[1], &pid, sizeof(pid));
+		if (ret != sizeof(pid)) {
+			/* If this really happens here, this is very unfortunate, since
+			 * the parent will not know the pid of the attached process and
+			 * will not be able to wait for it (and we won't either due to
+			 * CLONE_PARENT) so the parent won't be able to reap it and the
+			 * attached process will remain a zombie.
+			 */
+			shutdown(ipc_sockets[1], SHUT_RDWR);
+			lxc_proc_put_context_info(init_ctx);
+			_exit(EXIT_FAILURE);
+		}
+
+		TRACE("Sending pid %d of attached process", pid);
+
+		/* The rest is in the hands of the initial and the attached process. */
+		lxc_proc_put_context_info(init_ctx);
+		_exit(EXIT_SUCCESS);
+	}
+
+	to_cleanup_pid = pid;
 
 	/* close unneeded file descriptors */
 	close(ipc_sockets[1]);
-		free(cwd);
+	free_disarm(cwd);
 	lxc_proc_close_ns_fd(init_ctx);
 	if (options->attach_flags & LXC_ATTACH_TERMINAL)
 		lxc_attach_terminal_close_pts(&terminal);
@@ -1192,6 +1287,8 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 		ret = setup_proc_filesystem(&conf->procs, pid);
 		if (ret < 0)
 			goto on_error;
+
+		TRACE("Setup /proc/%d settings", pid);
 	}
 
 	/* Setup resource limits */
@@ -1199,6 +1296,8 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 		ret = setup_resource_limits(&conf->limits, pid);
 		if (ret < 0)
 			goto on_error;
+
+		TRACE("Setup resource limits");
 	}
 
 	if (options->attach_flags & LXC_ATTACH_TERMINAL) {
@@ -1242,9 +1341,8 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 
 	/* Open LSM fd and send it to child. */
 	if ((options->namespaces & CLONE_NEWNS) &&
-		    (options->attach_flags & LXC_ATTACH_LSM) &&
-		    init_ctx->lsm_label) {
-			int labelfd;
+	    (options->attach_flags & LXC_ATTACH_LSM) && init_ctx->lsm_label) {
+		__do_close int labelfd = -EBADF;
 		bool on_exec;
 
 		ret = -1;
@@ -1261,12 +1359,9 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 		if (ret <= 0) {
 			if (ret < 0)
 				SYSERROR("Failed to send lsm label fd");
-
-				close(labelfd);
 			goto close_mainloop;
 		}
 
-			close(labelfd);
 		TRACE("Sent LSM label file descriptor %d to child", labelfd);
 	}
 
@@ -1303,11 +1398,11 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 		}
 	}
 
-	close_mainloop:
+close_mainloop:
 	if (options->attach_flags & LXC_ATTACH_TERMINAL)
 		lxc_mainloop_close(&descr);
 
-	on_error:
+on_error:
 	if (ipc_sockets[0] >= 0) {
 		shutdown(ipc_sockets[0], SHUT_RDWR);
 		close(ipc_sockets[0]);
@@ -1323,107 +1418,6 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 
 	lxc_proc_put_context_info(init_ctx);
 	return ret_parent;
-	}
-
-	/* close unneeded file descriptors */
-	close_prot_errno_disarm(ipc_sockets[0]);
-
-	if (options->attach_flags & LXC_ATTACH_TERMINAL) {
-		lxc_attach_terminal_close_ptx(&terminal);
-		lxc_attach_terminal_close_peer(&terminal);
-		lxc_attach_terminal_close_log(&terminal);
-	}
-
-	/* Wait for the parent to have setup cgroups. */
-	ret = lxc_read_nointr(ipc_sockets[1], &status, sizeof(status));
-	if (ret != sizeof(status)) {
-		shutdown(ipc_sockets[1], SHUT_RDWR);
-		lxc_proc_put_context_info(init_ctx);
-		_exit(EXIT_FAILURE);
-	}
-
-	TRACE("Intermediate process starting to initialize");
-
-	/* Attach now, create another subprocess later, since pid namespaces
-	 * only really affect the children of the current process.
-	 */
-	ret = lxc_attach_to_ns(init_pid, init_ctx);
-	if (ret < 0) {
-		ERROR("Failed to enter namespaces");
-		shutdown(ipc_sockets[1], SHUT_RDWR);
-		lxc_proc_put_context_info(init_ctx);
-		_exit(EXIT_FAILURE);
-	}
-
-	/* close namespace file descriptors */
-	lxc_proc_close_ns_fd(init_ctx);
-
-	/* Attach succeeded, try to cwd. */
-	if (options->initial_cwd)
-		new_cwd = options->initial_cwd;
-	else
-		new_cwd = cwd;
-	if (new_cwd) {
-		ret = chdir(new_cwd);
-		if (ret < 0)
-			WARN("Could not change directory to \"%s\"", new_cwd);
-	}
-	free(cwd);
-
-	/* Create attached process. */
-	payload.ipc_socket = ipc_sockets[1];
-	payload.options = options;
-	payload.init_ctx = init_ctx;
-	payload.terminal_pts_fd = terminal.pty;
-	payload.exec_function = exec_function;
-	payload.exec_payload = exec_payload;
-
-	pid = lxc_raw_clone(CLONE_PARENT, NULL);
-	if (pid < 0) {
-		SYSERROR("Failed to clone attached process");
-		shutdown(ipc_sockets[1], SHUT_RDWR);
-		lxc_proc_put_context_info(init_ctx);
-		_exit(EXIT_FAILURE);
-	}
-
-	if (pid == 0) {
-		if (options->attach_flags & LXC_ATTACH_TERMINAL) {
-			ret = lxc_terminal_signal_sigmask_safe_blocked(&terminal);
-			if (ret < 0) {
-				SYSERROR("Failed to reset signal mask");
-				_exit(EXIT_FAILURE);
-			}
-		}
-
-		ret = attach_child_main(&payload);
-		if (ret < 0)
-			ERROR("Failed to exec");
-
-		_exit(EXIT_FAILURE);
-	}
-
-	if (options->attach_flags & LXC_ATTACH_TERMINAL)
-		lxc_attach_terminal_close_pts(&terminal);
-
-	/* Tell grandparent the pid of the pid of the newly created child. */
-	ret = lxc_write_nointr(ipc_sockets[1], &pid, sizeof(pid));
-	if (ret != sizeof(pid)) {
-		/* If this really happens here, this is very unfortunate, since
-		 * the parent will not know the pid of the attached process and
-		 * will not be able to wait for it (and we won't either due to
-		 * CLONE_PARENT) so the parent won't be able to reap it and the
-		 * attached process will remain a zombie.
-		 */
-		shutdown(ipc_sockets[1], SHUT_RDWR);
-		lxc_proc_put_context_info(init_ctx);
-		_exit(EXIT_FAILURE);
-	}
-
-	TRACE("Sending pid %d of attached process", pid);
-
-	/* The rest is in the hands of the initial and the attached process. */
-	lxc_proc_put_context_info(init_ctx);
-	_exit(EXIT_SUCCESS);
 }
 
 int lxc_attach_run_command(void *payload)