diff options
author | Dominique Martinet <asmadeus@codewreck.org> | 2018-07-01 00:00:15 +0900 |
---|---|---|
committer | Dominique Martinet <asmadeus@codewreck.org> | 2018-07-02 00:09:56 +0900 |
commit | ce17788533600bedf06cdbfbac1f2bd373484a24 (patch) | |
tree | a2499c87509c9650a543a5e9c6b838e6d8364af2 | |
parent | Merge pull request #2180 from martinetd/xdg_fullscreen (diff) | |
download | sway-ce17788533600bedf06cdbfbac1f2bd373484a24.tar.gz sway-ce17788533600bedf06cdbfbac1f2bd373484a24.tar.zst sway-ce17788533600bedf06cdbfbac1f2bd373484a24.zip |
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?!)
-rw-r--r-- | sway/commands/exec_always.c | 27 |
1 files changed, 13 insertions, 14 deletions
diff --git a/sway/commands/exec_always.c b/sway/commands/exec_always.c index 682d195e..1c99de97 100644 --- 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) { | |||
42 | wlr_log(L_ERROR, "Unable to create pipe for fork"); | 42 | wlr_log(L_ERROR, "Unable to create pipe for fork"); |
43 | } | 43 | } |
44 | 44 | ||
45 | pid_t pid; | 45 | pid_t pid, child; |
46 | pid_t *child = malloc(sizeof(pid_t)); // malloc'd so that Linux can avoid copying the process space | ||
47 | if (!child) { | ||
48 | return cmd_results_new(CMD_FAILURE, "exec_always", "Unable to allocate child pid"); | ||
49 | } | ||
50 | // Fork process | 46 | // Fork process |
51 | if ((pid = fork()) == 0) { | 47 | if ((pid = fork()) == 0) { |
52 | // Fork child process again | 48 | // Fork child process again |
53 | setsid(); | 49 | setsid(); |
54 | if ((*child = fork()) == 0) { | 50 | close(fd[0]); |
51 | if ((child = fork()) == 0) { | ||
52 | close(fd[1]); | ||
55 | execl("/bin/sh", "/bin/sh", "-c", cmd, (void *)NULL); | 53 | execl("/bin/sh", "/bin/sh", "-c", cmd, (void *)NULL); |
56 | // Not reached | 54 | _exit(0); |
57 | } | 55 | } |
58 | close(fd[0]); | ||
59 | ssize_t s = 0; | 56 | ssize_t s = 0; |
60 | while ((size_t)s < sizeof(pid_t)) { | 57 | while ((size_t)s < sizeof(pid_t)) { |
61 | s += write(fd[1], ((uint8_t *)child) + s, sizeof(pid_t) - s); | 58 | s += write(fd[1], ((uint8_t *)&child) + s, sizeof(pid_t) - s); |
62 | } | 59 | } |
63 | close(fd[1]); | 60 | close(fd[1]); |
64 | _exit(0); // Close child process | 61 | _exit(0); // Close child process |
65 | } else if (pid < 0) { | 62 | } else if (pid < 0) { |
66 | free(child); | 63 | close(fd[0]); |
64 | close(fd[1]); | ||
67 | return cmd_results_new(CMD_FAILURE, "exec_always", "fork() failed"); | 65 | return cmd_results_new(CMD_FAILURE, "exec_always", "fork() failed"); |
68 | } | 66 | } |
69 | close(fd[1]); // close write | 67 | close(fd[1]); // close write |
70 | ssize_t s = 0; | 68 | ssize_t s = 0; |
71 | while ((size_t)s < sizeof(pid_t)) { | 69 | while ((size_t)s < sizeof(pid_t)) { |
72 | s += read(fd[0], ((uint8_t *)child) + s, sizeof(pid_t) - s); | 70 | s += read(fd[0], ((uint8_t *)&child) + s, sizeof(pid_t) - s); |
73 | } | 71 | } |
74 | close(fd[0]); | 72 | close(fd[0]); |
75 | // cleanup child process | 73 | // cleanup child process |
76 | waitpid(pid, NULL, 0); | 74 | waitpid(pid, NULL, 0); |
77 | if (*child > 0) { | 75 | if (child > 0) { |
78 | wlr_log(L_DEBUG, "Child process created with pid %d", *child); | 76 | wlr_log(L_DEBUG, "Child process created with pid %d", child); |
79 | // TODO: add PID to active workspace | 77 | // TODO: add PID to active workspace |
80 | } else { | 78 | } else { |
81 | free(child); | 79 | return cmd_results_new(CMD_FAILURE, "exec_always", |
80 | "Second fork() failed"); | ||
82 | } | 81 | } |
83 | 82 | ||
84 | return cmd_results_new(CMD_SUCCESS, NULL, NULL); | 83 | return cmd_results_new(CMD_SUCCESS, NULL, NULL); |