process: block signals while spawning child processes

Between fork() and execvp() calls in the process_start()
function both child and parent processes share the same
file descriptors.  This means that, if a child process
received a signal during this time interval, then it could
potentially write data to a shared file descriptor.

One such example is fatal signal handler, where, if
child process received SIGTERM signal, then it would
write data into pipe.  Then a read event would occur
on the other end of the pipe where parent process is
listening and this would make parent process to incorrectly
believe that it was the one who received SIGTERM.
Also, since parent process never reads data from this
pipe, then this bug would make parent process to consume
100% CPU by immediately waking up from the event loop.

This patch will help to avoid this problem by blocking
signals until child closes all its file descriptors.

Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Reported-by: Suganya Ramachandran <suganyar@vmware.com>
Issue: 1255110

diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index ef3fbc0..3ddff00 100644 (file)
--- a/lib/fatal-signal.c
+++ b/lib/fatal-signal.c
@@ -374,3 +374,21 @@ fatal_signal_fork(void)
         raise(stored_sig_nr);
     }
 }
+
+#ifndef _WIN32
+/* Blocks all fatal signals and returns previous signal mask into
+ * 'prev_mask'. */
+void
+fatal_signal_block(sigset_t *prev_mask)
+{
+    int i;
+    sigset_t block_mask;
+
+    sigemptyset(&block_mask);
+    for (i = 0; i < ARRAY_SIZE(fatal_signals); i++) {
+        int sig_nr = fatal_signals[i];
+        sigaddset(&block_mask, sig_nr);
+    }
+    xpthread_sigmask(SIG_BLOCK, &block_mask, prev_mask);
+}
+#endif

diff --git a/lib/fatal-signal.h b/lib/fatal-signal.h
index caf24ec..e50d14a 100644 (file)
--- a/lib/fatal-signal.h
+++ b/lib/fatal-signal.h
@@ -17,6 +17,9 @@
 #ifndef FATAL_SIGNAL_H
 #define FATAL_SIGNAL_H 1
 
+#ifndef _WIN32
+#include <signal.h>
+#endif
 #include <stdbool.h>
 
 /* Basic interface. */
@@ -41,4 +44,8 @@ int fatal_signal_unlink_file_now(const char *);
  * it through. */
 void fatal_signal_handler(int sig_nr);
 
+#ifndef _WIN32
+void fatal_signal_block(sigset_t *prev_mask);
+#endif
+
 #endif /* fatal-signal.h */

diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index 49b3474..14db8df 100644 (file)
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -18,6 +18,9 @@
 #include "ovs-thread.h"
 #include <errno.h>
 #include <poll.h>
+#ifndef _WIN32
+#include <signal.h>
+#endif
 #include <stdlib.h>
 #include <unistd.h>
 #include "compiler.h"
@@ -177,6 +180,10 @@ XPTHREAD_FUNC2(pthread_key_create, pthread_key_t *, destructor_func *);
 XPTHREAD_FUNC1(pthread_key_delete, pthread_key_t);
 XPTHREAD_FUNC2(pthread_setspecific, pthread_key_t, const void *);
 
+#ifndef _WIN32
+XPTHREAD_FUNC3(pthread_sigmask, int, const sigset_t *, sigset_t *);
+#endif
+
 static void
 ovs_mutex_init__(const struct ovs_mutex *l_, int type)
 {

diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
index 68db71f..53bac23 100644 (file)
--- a/lib/ovs-thread.h
+++ b/lib/ovs-thread.h
@@ -157,6 +157,10 @@ void xpthread_key_create(pthread_key_t *, void (*destructor)(void *));
 void xpthread_key_delete(pthread_key_t);
 void xpthread_setspecific(pthread_key_t, const void *);
 
+#ifndef _WIN32
+void xpthread_sigmask(int, const sigset_t *, sigset_t *);
+#endif
+
 pthread_t ovs_thread_create(const char *name, void *(*)(void *), void *);
 void xpthread_join(pthread_t, void **);
 \f

diff --git a/lib/process.c b/lib/process.c
index 313f11f..7946d80 100644 (file)
--- a/lib/process.c
+++ b/lib/process.c
@@ -227,6 +227,7 @@ process_start(char **argv, struct process **pp)
 #ifndef _WIN32
     pid_t pid;
     int error;
+    sigset_t prev_mask;
 
     assert_single_threaded();
 
@@ -237,14 +238,15 @@ process_start(char **argv, struct process **pp)
         return error;
     }
 
+    fatal_signal_block(&prev_mask);
     pid = fork();
     if (pid < 0) {
         VLOG_WARN("fork failed: %s", ovs_strerror(errno));
-        return errno;
+        error = errno;
     } else if (pid) {
         /* Running in parent process. */
         *pp = process_register(argv[0], pid);
-        return 0;
+        error = 0;
     } else {
         /* Running in child process. */
         int fd_max = get_max_fds();
@@ -254,11 +256,14 @@ process_start(char **argv, struct process **pp)
         for (fd = 3; fd < fd_max; fd++) {
             close(fd);
         }
+        xpthread_sigmask(SIG_SETMASK, &prev_mask, NULL);
         execvp(argv[0], argv);
         fprintf(stderr, "execvp(\"%s\") failed: %s\n",
                 argv[0], ovs_strerror(errno));
         _exit(1);
     }
+    xpthread_sigmask(SIG_SETMASK, &prev_mask, NULL);
+    return error;
 #else
     *pp = NULL;
     return ENOSYS;