lib: make sure SEGV handler cannot lock up

Just adding -pthread to gcc options changes libc's behaviour, e.g.
making malloc() use proper locking.  This means a SEGV inside malloc()
(e.g. because malloc bookkeeping structures have been damaged by writing
to a broken pointer) can lead to a lockup by the following chain:

- random_function()
- malloc()
--- SEGV
- core_handler()
- zlog_backtrace_sigsafe()
- backtrace()
- malloc()

This will hang forever waiting for the malloc() lock to be released.

Another failure mode is dynamic linking with lazy binding (-z lazy,
default).  Since backtrace() is seldomly used, this means the call to
backtrace() in the core handler can in fact result in the dynamic linker
trying to resolve the "backtrace" symbol, which can also deadlock.

Add several safeguards to prevent any of this from happening.

(Unfortunately, these are not theoretical issues - I found them by
running into them headfirst.)

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/lib/log.c b/lib/log.c
index 69225dbf7ae1b08c5bda5a699395abc6c1c825c0..66320230ce84430600900291aa3deacb32d205d6 100644 (file)
--- a/lib/log.c
+++ b/lib/log.c
@@ -734,6 +734,17 @@ openzlog (const char *progname, const char *protoname, u_short instance,
 
   openlog (progname, syslog_flags, zl->facility);
   zlog_default = zl;
+
+#ifdef HAVE_GLIBC_BACKTRACE
+  /* work around backtrace() using lazily resolved dynamically linked
+   * symbols, which will otherwise cause funny breakage in the SEGV handler.
+   * (particularly, the dynamic linker can call malloc(), which uses locks
+   * in programs linked with -pthread, thus can deadlock.) */
+  void *bt[4];
+  backtrace (bt, array_size(bt));
+  free (backtrace_symbols (bt, 0));
+  backtrace_symbols_fd (bt, 0, 0);
+#endif
 }
 
 void

diff --git a/lib/sigevent.c b/lib/sigevent.c
index 09f07180cec884d406196ccdfd8959dd1335e6c9..b2059a17bf6104de35a54a847fa1f04c41fbbac7 100644 (file)
--- a/lib/sigevent.c
+++ b/lib/sigevent.c
@@ -233,6 +233,18 @@ core_handler(int signo
 #endif
            )
 {
+  /* make sure we don't hang in here.  default for SIGALRM is terminate.
+   * - if we're in backtrace for more than a second, abort. */
+  struct sigaction sa_default = { .sa_handler = SIG_DFL };
+  sigaction (SIGALRM, &sa_default, NULL);
+
+  sigset_t sigset;
+  sigemptyset (&sigset);
+  sigaddset (&sigset, SIGALRM);
+  sigprocmask (SIG_UNBLOCK, &sigset, NULL);
+
+  alarm (1);
+
   zlog_signal(signo, "aborting..."
 #ifdef SA_SIGINFO
              , siginfo, program_counter(context)
@@ -326,6 +338,11 @@ trap_default_signals(void)
 #else
                  act.sa_handler = sigmap[i].handler;
                  act.sa_flags = 0;
+#endif
+#ifdef SA_RESETHAND
+                  /* don't try to print backtraces recursively */
+                  if (sigmap[i].handler == core_handler)
+                    act.sa_flags |= SA_RESETHAND;
 #endif
                }
              if (sigaction(sigmap[i].sigs[j],&act,NULL) < 0)