Avoid using SIGUSR1 for interlock with ptrace-based intercept.

Instead, the child sleeping reading the intercept socketpair.  The tracer
writes to the other end of the socketpair after it has seized the child.
Fixes intercept/log_subcmds after the signal changes in 8781032.

Author: millert

src/exec.c

@@ -19,6 +19,7 @@
 #include <config.h>
 
 #include <sys/resource.h>
+#include <sys/socket.h>
 #include <sys/stat.h>
 #include <stdlib.h>
 #include <string.h>
@@ -43,14 +44,6 @@
 #include <sudo_plugin.h>
 #include <sudo_plugin_int.h>
 
-#ifdef HAVE_PTRACE_INTERCEPT
-static void
-handler(int signo)
-{
-    /* just return */
-}
-#endif /* HAVE_PTRACE_INTERCEPT */
-
 static void
 close_fds(struct command_details *details, int errfd, int intercept_fd)
 {
@@ -261,23 +254,11 @@ exec_cmnd(struct command_details *details, sigset_t *mask,
 
 #ifdef HAVE_PTRACE_INTERCEPT
     if (ISSET(details->flags, CD_USE_PTRACE)) {
-	struct sigaction sa;
-	sigset_t set;
-
-	/* Tracer will send us SIGUSR1 when it is time to proceed. */
-	memset(&sa, 0, sizeof(sa));
-	sigemptyset(&sa.sa_mask);
-	sa.sa_flags = SA_RESTART;
-	sa.sa_handler = handler;
-	if (sudo_sigaction(SIGUSR1, &sa, NULL) != 0) {
-	    sudo_warn(U_("unable to set handler for signal %d"),
-		SIGUSR1);
-	}
+	struct command_status cstat;
 
-	/* Suspend child until tracer seizes control and sends SIGUSR1. */
-	sigfillset(&set);
-	sigdelset(&set, SIGUSR1);
-	sigsuspend(&set);
+	/* Wait for tracer to seizes control and notify us. */
+	if (recv(intercept_fd, &cstat, sizeof(cstat), MSG_WAITALL) == -1)
+	    sudo_warn("%s", U_("unable to receive message from parent"));
     }
 #endif /* HAVE_PTRACE_INTERCEPT */
 

src/exec_monitor.c

@@ -620,9 +620,10 @@ exec_monitor(struct command_details *details, sigset_t *oset,
      * Create new event base and register read events for the
      * signal pipe, error pipe, and backchannel.
      */
-     init_exec_events_monitor(&mc, errsock[0]);
-     /* Restore signal mask now that signal handlers are setup. */
-     sigprocmask(SIG_SETMASK, oset, NULL);
+    init_exec_events_monitor(&mc, errsock[0]);
+
+    /* Restore signal mask now that signal handlers are setup. */
+    sigprocmask(SIG_SETMASK, oset, NULL);
 
     mc.cmnd_pid = sudo_debug_fork();
     switch (mc.cmnd_pid) {

src/exec_nopty.c

@@ -569,13 +569,19 @@ exec_nopty(struct command_details *details,
 	sudo_fatal("%s", U_("unable to create pipe"));
 
     if (ISSET(details->flags, CD_INTERCEPT|CD_LOG_SUBCMDS)) {
-	if (!ISSET(details->flags, CD_USE_PTRACE)) {
-	    /*
-	     * Allocate a socketpair for communicating with sudo_intercept.so.
-	     * This must be inherited across exec, hence no FD_CLOEXEC.
-	     */
-	    if (socketpair(PF_UNIX, SOCK_STREAM, 0, intercept_sv) == -1)
+	/*
+	 * Allocate a socketpair for communicating with sudo_intercept.so.
+	 * This must be inherited across exec for the non-ptrace case.
+	 */
+	if (socketpair(PF_UNIX, SOCK_STREAM, 0, intercept_sv) == -1)
+	    sudo_fatal("%s", U_("unable to create sockets"));
+	if (ISSET(details->flags, CD_USE_PTRACE)) {
+	    if (fcntl(intercept_sv[0], F_SETFD, FD_CLOEXEC) == -1 ||
+		    fcntl(intercept_sv[1], F_SETFD, FD_CLOEXEC) == -1) {
 		sudo_fatal("%s", U_("unable to create sockets"));
+	    }
+	    /* Used to wake up command as it waits for parent to seize it. */
+	    ec.intercept_fd = intercept_sv[0];
 	}
     }
 
@@ -608,8 +614,10 @@ exec_nopty(struct command_details *details,
 
     /* Allocate and set signal events and the error pipe event.*/
     init_exec_events(&ec, evbase, errpipe[0]);
+
     /* Restore signal mask now that signal handlers are setup. */
     sigprocmask(SIG_SETMASK, &oset, NULL);
+
     ec.cmnd_pid = sudo_debug_fork();
     switch (ec.cmnd_pid) {
     case -1:
@@ -680,7 +688,7 @@ exec_nopty(struct command_details *details,
 	    rc = -1;
 	} else if (ISSET(details->flags, CD_USE_PTRACE)) {
 	    /* Try to seize control of the command using ptrace(2). */
-	    rc = exec_ptrace_seize(ec.cmnd_pid);
+	    rc = exec_ptrace_seize(ec.cmnd_pid, intercept_sv[0]);
 	    if (rc == 0) {
 		/* There is another tracer present. */
 		CLR(details->flags, CD_INTERCEPT|CD_LOG_SUBCMDS|CD_USE_PTRACE);

src/exec_ptrace.c

@@ -16,6 +16,7 @@
 
 #include <config.h>
 
+#include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/uio.h>
@@ -1228,11 +1229,12 @@ set_exec_filter(void)
  * being traced and -1 on error.
  */
 int
-exec_ptrace_seize(pid_t child)
+exec_ptrace_seize(pid_t child, int intercept_fd)
 {
     const long ptrace_opts = PTRACE_O_TRACESECCOMP|PTRACE_O_TRACECLONE|
 			     PTRACE_O_TRACEFORK|PTRACE_O_TRACEVFORK|
 			     PTRACE_O_TRACEEXEC;
+    struct command_status cstat;
     int ret = -1;
     int status;
     debug_decl(exec_ptrace_seize, SUDO_DEBUG_EXEC);
@@ -1264,11 +1266,15 @@ exec_ptrace_seize(pid_t child)
 	ret = false;
     }
 
-    /* The child is suspended waiting for SIGUSR1, wake it up. */
-    if (kill(child, SIGUSR1) == -1) {
-	sudo_warn("kill(%d, SIGUSR1)", (int)child);
+    /* The child is waiting on intercept_fd, wake it up. */
+    cstat.type = CMD_SIGNO;
+    cstat.val = 0;
+    if (send(intercept_fd, &cstat, sizeof(cstat), 0) == -1) {
+	sudo_warn("%s", U_("unable to wake up child"));
 	goto done;
     }
+
+    /* If PTRACE_SEIZE failed, we are done. */
     if (!ret)
 	goto done;
 
@@ -2100,7 +2106,7 @@ exec_ptrace_stopped(pid_t pid, int status, void *intercept)
 
 /* STUB */
 int
-exec_ptrace_seize(pid_t child)
+exec_ptrace_seize(pid_t child, int intercept_fd)
 {
     return true;
 }

src/exec_pty.c

@@ -716,7 +716,7 @@ backchannel_cb(int fd, int what, void *v)
 		ec->details->command, (int)ec->cmnd_pid);
 	    if (ISSET(ec->details->flags, CD_USE_PTRACE)) {
 		/* Try to seize control of the command using ptrace(2). */
-		int rc = exec_ptrace_seize(ec->cmnd_pid);
+		const int rc = exec_ptrace_seize(ec->cmnd_pid, ec->intercept_fd);
 		if (rc == 0) {
 		    /* There is another tracer present. */
 		    CLR(ec->details->flags, CD_INTERCEPT|CD_LOG_SUBCMDS|CD_USE_PTRACE);
@@ -974,6 +974,7 @@ init_exec_closure(struct exec_closure *ec, struct command_status *cstat,
     ec->cmnd_pid = -1;
     ec->cstat = cstat;
     ec->details = details;
+    ec->intercept_fd = -1;
     ec->rows = user_details->ts_rows;
     ec->cols = user_details->ts_cols;
 
@@ -1135,13 +1136,19 @@ exec_pty(struct command_details *details,
 	sudo_fatal("%s", U_("unable to create sockets"));
 
     if (ISSET(details->flags, CD_INTERCEPT|CD_LOG_SUBCMDS)) {
-	if (!ISSET(details->flags, CD_USE_PTRACE)) {
-	    /*
-	     * Allocate a socketpair for communicating with sudo_intercept.so.
-	     * This must be inherited across exec, hence no FD_CLOEXEC.
-	     */
-	    if (socketpair(PF_UNIX, SOCK_STREAM, 0, intercept_sv) == -1)
+	/*
+	 * Allocate a socketpair for communicating with sudo_intercept.so.
+	 * This must be inherited across exec for the non-ptrace case.
+	 */
+	if (socketpair(PF_UNIX, SOCK_STREAM, 0, intercept_sv) == -1)
+	    sudo_fatal("%s", U_("unable to create sockets"));
+        if (ISSET(details->flags, CD_USE_PTRACE)) {
+	    if (fcntl(intercept_sv[0], F_SETFD, FD_CLOEXEC) == -1 ||
+		    fcntl(intercept_sv[1], F_SETFD, FD_CLOEXEC) == -1) {
 		sudo_fatal("%s", U_("unable to create sockets"));
+	    }
+	    /* Used to wake up command as it waits for parent to seize it. */
+	    ec->intercept_fd = intercept_sv[0];
 	}
     }
 
@@ -1337,8 +1344,10 @@ exec_pty(struct command_details *details,
 
     /* Allocate and set signal events and the backchannel event. */
     init_exec_events(ec, evbase, sv[0]);
+
     /* Restore signal mask now that signal handlers are setup. */
     sigprocmask(SIG_SETMASK, &oset, NULL);
+
     ec->monitor_pid = sudo_debug_fork();
     switch (ec->monitor_pid) {
     case -1:

src/regress/intercept/test_ptrace.c

@@ -40,7 +40,10 @@ handler(int signo)
 void
 intercept_closure_reset(struct intercept_closure *closure)
 {
+    /* Preserve closure->details, zero everything else. */
+    const struct command_details *details = closure->details;
     memset(closure, 0, sizeof(*closure));
+    closure->details = details;
 }
 
 bool
@@ -106,6 +109,8 @@ main(int argc, char *argv[])
     struct sudo_debug_file debug_file;
     const char *base, *shell = _PATH_SUDO_BSHELL;
     struct intercept_closure closure = { 0 };
+    struct command_details details = { 0 };
+    int intercept_sv[2];
     const char *errstr;
     sigset_t blocked, empty;
     struct sigaction sa;
@@ -145,20 +150,29 @@ main(int argc, char *argv[])
     if (sudo_debug_instance == SUDO_DEBUG_INSTANCE_ERROR)
 	return EXIT_FAILURE;
 
-    /* Block SIGCHLD and SIGUSR during critical section. */
+    /* Use socketpair to coordinate between parent and child. */
+    if (socketpair(PF_UNIX, SOCK_STREAM, 0, intercept_sv) == -1)
+	sudo_fatal("%s", U_("unable to create sockets"));
+    if (fcntl(intercept_sv[0], F_SETFD, FD_CLOEXEC) == -1 ||
+	    fcntl(intercept_sv[1], F_SETFD, FD_CLOEXEC) == -1) {
+	sudo_fatal("%s", U_("unable to create sockets"));
+    }
+
+    /* Block SIGCHLD during critical section. */
     sigemptyset(&empty);
     sigemptyset(&blocked);
     sigaddset(&blocked, SIGCHLD);
-    sigaddset(&blocked, SIGUSR1);
     sigprocmask(SIG_BLOCK, &blocked, NULL);
 
-    /* Signal handler sets a flag for SIGCHLD, nothing for SIGUSR1. */
+    /* Avoid NULL deref of closure.details. */
+    closure.details = &details;
+
+    /* Signal handler sets a flag for SIGCHLD. */
     memset(&sa, 0, sizeof(sa));
     sigemptyset(&sa.sa_mask);
     sa.sa_flags = SA_RESTART;
     sa.sa_handler = handler;
     sigaction(SIGCHLD, &sa, NULL);
-    sigaction(SIGUSR1, &sa, NULL);
 
     /* Fork a shell. */
     my_pid = getpid();
@@ -174,13 +188,16 @@ main(int argc, char *argv[])
 	if (!set_exec_filter())
 	    _exit(EXIT_FAILURE);
 
-	/* Suspend child until tracer seizes control and sends SIGUSR1. */
-	sigsuspend(&empty);
+	/* Child waits until tracer seizes control and sends SIGUSR1. */
+	close(intercept_sv[0]);
+	recv(intercept_sv[1], &ch, sizeof(ch), 0);
+
 	execl(shell, base, NULL);
 	sudo_fatal("execl");
     default:
 	/* Parent attaches to child and allows it to continue. */
-	if (exec_ptrace_seize(child) == -1)
+	close(intercept_sv[1]);
+	if (exec_ptrace_seize(child, intercept_sv[0]) == -1)
 	    return EXIT_FAILURE;
 	break;
     }

src/sudo_exec.h

@@ -76,6 +76,7 @@ struct exec_closure {
     pid_t monitor_pid;
     pid_t cmnd_pid;
     pid_t ppgrp;
+    int intercept_fd;
     int rows;
     int cols;
     bool foreground;
@@ -228,7 +229,7 @@ char **sudo_preload_dso_mmap(char *const envp[], const char *dso_file, int inter
 /* exec_ptrace.c */
 bool exec_ptrace_stopped(pid_t pid, int status, void *intercept);
 bool set_exec_filter(void);
-int exec_ptrace_seize(pid_t child);
+int exec_ptrace_seize(pid_t child, int intercept_fd);
 
 /* suspend_parent.c */
 void sudo_suspend_parent(int signo, pid_t my_pid, pid_t my_pgrp, pid_t cmnd_pid, void *closure, void (*callback)(void *, int));