Add tcsetpgrp_nobg() which acts like tcsetpgrp() but returns -1

for a background process.  This is safer than blocking SIGTTOU
which would cause tcsetpgrp() to succeed in the background.

diff --git a/MANIFEST b/MANIFEST
index 6d487c73ac4d35edc44913261528d7cfd476d2a5..bf9b19346d643c26d0ac4b52dc6776b6bd2d957e 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -618,6 +618,7 @@ src/sudo_exec.h
 src/sudo_noexec.c
 src/sudo_plugin_int.h
 src/sudo_usage.h.in
+src/tcsetpgrp_nobg.c
 src/tgetpass.c
 src/ttyname.c
 src/utmp.c

diff --git a/src/Makefile.in b/src/Makefile.in
index b7f13d8c72e462b5f5f0495a7e3d84a84172cdb0..607d6ae4b46420207391ef18a2159da46575f0e2 100644 (file)
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -116,8 +116,8 @@ PROGS = @PROGS@
 
 OBJS = conversation.o env_hooks.o exec.o exec_common.o exec_monitor.o \
        exec_nopty.o exec_pty.o get_pty.o hooks.o net_ifs.o load_plugins.o \
-       parse_args.o preserve_fds.o signal.o sudo.o sudo_edit.o tgetpass.o \
-       ttyname.o utmp.o @SUDO_OBJS@
+       parse_args.o preserve_fds.o signal.o sudo.o sudo_edit.o \
+       tcsetpgrp_nobg.o tgetpass.o ttyname.o utmp.o @SUDO_OBJS@
 
 SESH_OBJS = sesh.o exec_common.o
 
@@ -421,6 +421,13 @@ sudo_edit.o: $(srcdir)/sudo_edit.c $(incdir)/compat/stdbool.h \
              $(incdir)/sudo_util.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \
              $(top_builddir)/config.h $(top_builddir)/pathnames.h
        $(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(SSP_CFLAGS) $(srcdir)/sudo_edit.c
+tcsetpgrp_nobg.o: $(srcdir)/tcsetpgrp_nobg.c $(incdir)/compat/stdbool.h \
+                  $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \
+                  $(incdir)/sudo_debug.h $(incdir)/sudo_fatal.h \
+                  $(incdir)/sudo_gettext.h $(incdir)/sudo_queue.h \
+                  $(incdir)/sudo_util.h $(srcdir)/sudo.h \
+                  $(top_builddir)/config.h $(top_builddir)/pathnames.h
+       $(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(SSP_CFLAGS) $(srcdir)/tcsetpgrp_nobg.c
 tgetpass.o: $(srcdir)/tgetpass.c $(incdir)/compat/stdbool.h \
             $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \
             $(incdir)/sudo_debug.h $(incdir)/sudo_fatal.h \

diff --git a/src/exec_nopty.c b/src/exec_nopty.c
index b6826ef70e9d0ec463c30b086a278a21e0e403ab..0fb72f94e643892b8565415f9beb53235612bead 100644 (file)
--- a/src/exec_nopty.c
+++ b/src/exec_nopty.c
@@ -394,18 +394,12 @@ dispatch_signal(struct exec_closure_nopty *ec, int signo, char *signame)
                     * Restore foreground process group, if different.
                     * Otherwise, we cannot resume some shells (pdksh).
                     *
-                    * There appears to be a race with shells that do restore
-                    * the controlling group.  Sudo can receive SIGTTOU
-                    * if the shell has already changed the controlling tty.
+                    * It is possible that we are no longer the foreground
+                    * process so use tcsetpgrp_nobg() to avoid sudo
+                    * receiving SIGTTOU.
                     */
-                   if (saved_pgrp != ppgrp) {
-                       sigset_t set, oset;
-                       sigemptyset(&set);
-                       sigaddset(&set, SIGTTOU);
-                       sigprocmask(SIG_BLOCK, &set, &oset);
-                       (void)tcsetpgrp(fd, saved_pgrp);
-                       sigprocmask(SIG_SETMASK, &oset, NULL);
-                   }
+                   if (saved_pgrp != ppgrp)
+                       tcsetpgrp_nobg(fd, saved_pgrp);
                    close(fd);
                }
            } else {

diff --git a/src/sudo.h b/src/sudo.h
index 3ac2c9db41ac1bca3b62d6ab17accffc4110192a..43175b68028f53aa95ef2c3cb85516aca877208a 100644 (file)
--- a/src/sudo.h
+++ b/src/sudo.h
@@ -271,4 +271,7 @@ int add_preserved_fd(struct preserved_fd_list *pfds, int fd);
 void closefrom_except(int startfd, struct preserved_fd_list *pfds);
 void parse_preserved_fds(struct preserved_fd_list *pfds, const char *fdstr);
 
+/* setpgrp_nobg.c */
+int tcsetpgrp_nobg(int fd, pid_t pgrp_id);
+
 #endif /* SUDO_SUDO_H */

diff --git a/src/tcsetpgrp_nobg.c b/src/tcsetpgrp_nobg.c
new file mode 100644 (file)
index 0000000..7179fc7
--- /dev/null
+++ b/src/tcsetpgrp_nobg.c
@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2017 Todd C. Miller <Todd.Miller@courtesan.com>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <config.h>
+
+#include <sys/types.h>
+#include <stdio.h>
+#include <stdlib.h>
+#ifdef HAVE_STRING_H
+# include <string.h>
+#endif /* HAVE_STRING_H */
+#ifdef HAVE_STRINGS_H
+# include <strings.h>
+#endif /* HAVE_STRINGS_H */
+#include <unistd.h>
+#include <errno.h>
+#include <signal.h>
+
+#include "sudo.h"
+
+static volatile sig_atomic_t got_sigttou;
+
+/*
+ * SIGTTOU signal handler for tcsetpgrp_nobg() that just sets a flag.
+ */
+static void
+sigttou(int signo)
+{
+    got_sigttou = 1;
+}
+
+/*
+ * Like tcsetpgrp() but restarts on EINTR _except_ for SIGTTOU.
+ * Returns 0 on success or -1 on failure, setting errno.
+ * Sets got_sigttou on failure if interrupted by SIGTTOU.
+ */
+int
+tcsetpgrp_nobg(int fd, pid_t pgrp_id)
+{
+    sigaction_t sa, osa;
+    int rc;
+
+    /*
+     * If we receive SIGTTOU from tcsetpgrp() it means we are
+     * not in the foreground process group.
+     * This avoid a TOCTOU race compared to using tcgetpgrp().
+     */
+    memset(&sa, 0, sizeof(sa));
+    sigemptyset(&sa.sa_mask);
+    sa.sa_flags = SA_INTERRUPT; /* do not restart syscalls */
+    sa.sa_handler = sigttou;
+    got_sigttou = 0;
+    (void)sigaction(SIGTTOU, &sa, &osa);
+    do {
+       rc = tcsetpgrp(fd, pgrp_id);
+    } while (rc != 0 && errno == EINTR && !got_sigttou);
+    (void)sigaction(SIGTTOU, &osa, NULL);
+
+    return rc;
+}