sway

commit ce17788533600bedf06cdbfbac1f2bd373484a24
parent 62a7b762aca206c0992bda51153fd0cfb4f41636
Author: Dominique Martinet <asmadeus@codewreck.org>
Date:   Sun,  1 Jul 2018 00:00:15 +0900

exec_always: fix leaks

 - child would leak in the workspace_record_pid path
 - removing malloc lets us get rid of That Comment nobody seems
to remember what it was about
 - we would leak pipe fds on first fork failling
 - we didn't return an error if second fork failed
 - the final executed process still had both pipe fds
(would show up in /proc/23560/fd in launched programs)
 - we would write twice to the pipe if execl failed for some reason
(e.g. if /bin/sh doesn't exist?!)

Diffstat:
M
sway/commands/exec_always.c
 | 
27
+++++++++++++--------------

1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/sway/commands/exec_always.c b/sway/commands/exec_always.c
@@ -42,43 +42,42 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) {
 		wlr_log(L_ERROR, "Unable to create pipe for fork");
 	}
 
-	pid_t pid;
-	pid_t *child = malloc(sizeof(pid_t)); // malloc'd so that Linux can avoid copying the process space
-	if (!child) {
-		return cmd_results_new(CMD_FAILURE, "exec_always", "Unable to allocate child pid");
-	}
+	pid_t pid, child;
 	// Fork process
 	if ((pid = fork()) == 0) {
 		// Fork child process again
 		setsid();
-		if ((*child = fork()) == 0) {
+		close(fd[0]);
+		if ((child = fork()) == 0) {
+			close(fd[1]);
 			execl("/bin/sh", "/bin/sh", "-c", cmd, (void *)NULL);
-			// Not reached
+			_exit(0);
 		}
-		close(fd[0]);
 		ssize_t s = 0;
 		while ((size_t)s < sizeof(pid_t)) {
-			s += write(fd[1], ((uint8_t *)child) + s, sizeof(pid_t) - s);
+			s += write(fd[1], ((uint8_t *)&child) + s, sizeof(pid_t) - s);
 		}
 		close(fd[1]);
 		_exit(0); // Close child process
 	} else if (pid < 0) {
-		free(child);
+		close(fd[0]);
+		close(fd[1]);
 		return cmd_results_new(CMD_FAILURE, "exec_always", "fork() failed");
 	}
 	close(fd[1]); // close write
 	ssize_t s = 0;
 	while ((size_t)s < sizeof(pid_t)) {
-		s += read(fd[0], ((uint8_t *)child) + s, sizeof(pid_t) - s);
+		s += read(fd[0], ((uint8_t *)&child) + s, sizeof(pid_t) - s);
 	}
 	close(fd[0]);
 	// cleanup child process
 	waitpid(pid, NULL, 0);
-	if (*child > 0) {
-		wlr_log(L_DEBUG, "Child process created with pid %d", *child);
+	if (child > 0) {
+		wlr_log(L_DEBUG, "Child process created with pid %d", child);
 		// TODO: add PID to active workspace
 	} else {
-		free(child);
+		return cmd_results_new(CMD_FAILURE, "exec_always",
+			"Second fork() failed");
 	}
 
 	return cmd_results_new(CMD_SUCCESS, NULL, NULL);