Block signals between forking and clearing signal handlers (or calling

event_reinit) - if the child gets a signal and fires the libevent signal
handler during this period it could write a signal into the parent's
signal pipe. GitHub issue 1001 from Aaron van Geffen.
This commit is contained in:
nicm 2017-07-12 10:04:51 +00:00
parent 0453ad0146
commit 51112221ee
4 changed files with 33 additions and 9 deletions

View File

@ -22,6 +22,7 @@
#include <errno.h> #include <errno.h>
#include <fcntl.h> #include <fcntl.h>
#include <paths.h> #include <paths.h>
#include <signal.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <time.h> #include <time.h>
@ -62,6 +63,7 @@ cmd_pipe_pane_exec(struct cmd *self, struct cmdq_item *item)
char *cmd; char *cmd;
int old_fd, pipe_fd[2], null_fd; int old_fd, pipe_fd[2], null_fd;
struct format_tree *ft; struct format_tree *ft;
sigset_t set, oldset;
/* Destroy the old pipe. */ /* Destroy the old pipe. */
old_fd = wp->pipe_fd; old_fd = wp->pipe_fd;
@ -102,16 +104,20 @@ cmd_pipe_pane_exec(struct cmd *self, struct cmdq_item *item)
format_free(ft); format_free(ft);
/* Fork the child. */ /* Fork the child. */
sigfillset(&set);
sigprocmask(SIG_BLOCK, &set, &oldset);
switch (fork()) { switch (fork()) {
case -1: case -1:
sigprocmask(SIG_SETMASK, &oldset, NULL);
cmdq_error(item, "fork error: %s", strerror(errno)); cmdq_error(item, "fork error: %s", strerror(errno));
free(cmd); free(cmd);
return (CMD_RETURN_ERROR); return (CMD_RETURN_ERROR);
case 0: case 0:
/* Child process. */ /* Child process. */
close(pipe_fd[0]);
proc_clear_signals(server_proc); proc_clear_signals(server_proc);
sigprocmask(SIG_SETMASK, &oldset, NULL);
close(pipe_fd[0]);
if (dup2(pipe_fd[1], STDIN_FILENO) == -1) if (dup2(pipe_fd[1], STDIN_FILENO) == -1)
_exit(1); _exit(1);
@ -132,6 +138,7 @@ cmd_pipe_pane_exec(struct cmd *self, struct cmdq_item *item)
_exit(1); _exit(1);
default: default:
/* Parent process. */ /* Parent process. */
sigprocmask(SIG_SETMASK, &oldset, NULL);
close(pipe_fd[1]); close(pipe_fd[1]);
wp->pipe_fd = pipe_fd[0]; wp->pipe_fd = pipe_fd[0];

9
job.c
View File

@ -51,6 +51,7 @@ job_run(const char *cmd, struct session *s, const char *cwd,
pid_t pid; pid_t pid;
int nullfd, out[2]; int nullfd, out[2];
const char *home; const char *home;
sigset_t set, oldset;
if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, out) != 0) if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, out) != 0)
return (NULL); return (NULL);
@ -61,14 +62,18 @@ job_run(const char *cmd, struct session *s, const char *cwd,
*/ */
env = environ_for_session(s, !cfg_finished); env = environ_for_session(s, !cfg_finished);
sigfillset(&set);
sigprocmask(SIG_BLOCK, &set, &oldset);
switch (pid = fork()) { switch (pid = fork()) {
case -1: case -1:
sigprocmask(SIG_SETMASK, &oldset, NULL);
environ_free(env); environ_free(env);
close(out[0]); close(out[0]);
close(out[1]); close(out[1]);
return (NULL); return (NULL);
case 0: /* child */ case 0:
proc_clear_signals(server_proc); proc_clear_signals(server_proc);
sigprocmask(SIG_SETMASK, &oldset, NULL);
if (cwd == NULL || chdir(cwd) != 0) { if (cwd == NULL || chdir(cwd) != 0) {
if ((home = find_home()) == NULL || chdir(home) != 0) if ((home = find_home()) == NULL || chdir(home) != 0)
@ -100,7 +105,7 @@ job_run(const char *cmd, struct session *s, const char *cwd,
fatal("execl failed"); fatal("execl failed");
} }
/* parent */ sigprocmask(SIG_SETMASK, &oldset, NULL);
environ_free(env); environ_free(env);
close(out[1]); close(out[1]);

View File

@ -141,21 +141,24 @@ server_start(struct tmuxproc *client, struct event_base *base, int lockfd,
{ {
int pair[2]; int pair[2];
struct job *job; struct job *job;
sigset_t set, oldset;
if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pair) != 0) if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pair) != 0)
fatal("socketpair failed"); fatal("socketpair failed");
sigfillset(&set);
sigprocmask(SIG_BLOCK, &set, &oldset);
switch (fork()) { switch (fork()) {
case -1: case -1:
fatal("fork failed"); fatal("fork failed");
case 0: case 0:
break; break;
default: default:
sigprocmask(SIG_SETMASK, &oldset, NULL);
close(pair[1]); close(pair[1]);
return (pair[0]); return (pair[0]);
} }
close(pair[0]); close(pair[0]);
if (daemon(1, 0) != 0) if (daemon(1, 0) != 0)
fatal("daemon failed"); fatal("daemon failed");
proc_clear_signals(client); proc_clear_signals(client);
@ -163,6 +166,7 @@ server_start(struct tmuxproc *client, struct event_base *base, int lockfd,
fatalx("event_reinit failed"); fatalx("event_reinit failed");
server_proc = proc_start("server"); server_proc = proc_start("server");
proc_set_signals(server_proc, server_signal); proc_set_signals(server_proc, server_signal);
sigprocmask(SIG_SETMASK, &oldset, NULL);
if (log_get_level() > 1) if (log_get_level() > 1)
tty_create_log(); tty_create_log();

View File

@ -22,6 +22,7 @@
#include <errno.h> #include <errno.h>
#include <fcntl.h> #include <fcntl.h>
#include <fnmatch.h> #include <fnmatch.h>
#include <signal.h>
#include <stdint.h> #include <stdint.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
@ -886,6 +887,7 @@ window_pane_spawn(struct window_pane *wp, int argc, char **argv,
const char *ptr, *first, *home; const char *ptr, *first, *home;
struct termios tio2; struct termios tio2;
int i; int i;
sigset_t set, oldset;
if (wp->fd != -1) { if (wp->fd != -1) {
bufferevent_free(wp->event); bufferevent_free(wp->event);
@ -915,14 +917,21 @@ window_pane_spawn(struct window_pane *wp, int argc, char **argv,
ws.ws_col = screen_size_x(&wp->base); ws.ws_col = screen_size_x(&wp->base);
ws.ws_row = screen_size_y(&wp->base); ws.ws_row = screen_size_y(&wp->base);
wp->pid = fdforkpty(ptm_fd, &wp->fd, wp->tty, NULL, &ws); sigfillset(&set);
switch (wp->pid) { sigprocmask(SIG_BLOCK, &set, &oldset);
switch (wp->pid = fdforkpty(ptm_fd, &wp->fd, wp->tty, NULL, &ws)) {
case -1: case -1:
wp->fd = -1; wp->fd = -1;
xasprintf(cause, "%s: %s", cmd, strerror(errno)); xasprintf(cause, "%s: %s", cmd, strerror(errno));
free(cmd); free(cmd);
sigprocmask(SIG_SETMASK, &oldset, NULL);
return (-1); return (-1);
case 0: case 0:
proc_clear_signals(server_proc);
sigprocmask(SIG_SETMASK, &oldset, NULL);
if (chdir(wp->cwd) != 0) { if (chdir(wp->cwd) != 0) {
if ((home = find_home()) == NULL || chdir(home) != 0) if ((home = find_home()) == NULL || chdir(home) != 0)
chdir("/"); chdir("/");
@ -936,6 +945,7 @@ window_pane_spawn(struct window_pane *wp, int argc, char **argv,
if (tcsetattr(STDIN_FILENO, TCSANOW, &tio2) != 0) if (tcsetattr(STDIN_FILENO, TCSANOW, &tio2) != 0)
fatal("tcgetattr failed"); fatal("tcgetattr failed");
log_close();
closefrom(STDERR_FILENO + 1); closefrom(STDERR_FILENO + 1);
if (path != NULL) if (path != NULL)
@ -943,9 +953,6 @@ window_pane_spawn(struct window_pane *wp, int argc, char **argv,
environ_set(env, "TMUX_PANE", "%%%u", wp->id); environ_set(env, "TMUX_PANE", "%%%u", wp->id);
environ_push(env); environ_push(env);
proc_clear_signals(server_proc);
log_close();
setenv("SHELL", wp->shell, 1); setenv("SHELL", wp->shell, 1);
ptr = strrchr(wp->shell, '/'); ptr = strrchr(wp->shell, '/');
@ -978,6 +985,7 @@ window_pane_spawn(struct window_pane *wp, int argc, char **argv,
fatal("execl failed"); fatal("execl failed");
} }
sigprocmask(SIG_SETMASK, &oldset, NULL);
setblocking(wp->fd, 0); setblocking(wp->fd, 0);
wp->event = bufferevent_new(wp->fd, window_pane_read_callback, NULL, wp->event = bufferevent_new(wp->fd, window_pane_read_callback, NULL,