[PATCH] os: add FatalErrorSigSafe for signal-safe logging

[Peter Hutterer]

Backtrace logging etc. is already sigsafe, but the actual FatalError message
in response is not yet, leading to amusing logs like this:

    (EE) Segmentation fault at address 0x0
    (EE) BUG: triggered 'if (inSignalContext)'
    (EE) BUG: log.c:499 in LogVMessageVerb()
    (EE) Warning: attempting to log data in a signal unsafe manner while in
    signal context.
    Please update to check inSignalContext and/or use LogMessageVerbSigSafe() or
    ErrorFSigSafe().
    The offending log format message is:

    Fatal server error:

Ideally we could just use ErrorFSigSafe from FatalError but pnprintf does
not yet support all printf directives used in the server by the various
FatalError commands.

Since we're late in the release cycle, fix the most visible part of the
problem only. Add a new FatalErrorSigSafe and print that in sigsafe-manner.
Only the signal handler instance is changed, the rest of the server keeps
using the old one.

Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
It's not nice, but extending pnprintf is a rabbit hole and I'd rather not
introduce a new printf-related bug this close to a release.

 include/os.h |  6 ++++++
 os/log.c     | 40 ++++++++++++++++++++++++++++++++--------
 os/osinit.c  |  4 ++--
 3 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/include/os.h b/include/os.h
index c7108a5..2bdbf95 100644
--- a/include/os.h
+++ b/include/os.h
@@ -655,6 +655,12 @@ FatalError(const char *f, ...)
 _X_ATTRIBUTE_PRINTF(1, 2)
     _X_NORETURN;
 
+/* XXX: DO NOT USE! Temporary */
+extern _X_EXPORT void
+FatalErrorSigSafe(const char *f, ...)
+_X_ATTRIBUTE_PRINTF(1, 2)
+    _X_NORETURN;
+
 #ifdef DEBUG
 #define DebugF ErrorF
 #else
diff --git a/os/log.c b/os/log.c
index 95bd8cc..1d89851 100644
--- a/os/log.c
+++ b/os/log.c
@@ -773,19 +773,43 @@ VAuditF(const char *f, va_list args)
     free(prefix);
 }
 
-void
-FatalError(const char *f, ...)
+/* FIXME: instead of using FatalErrorSigSafe, make pnprintf deal with the
+   various printf directives used in FatalError across the server, then use
+   ErrorFSigSafe directly.
+ */
+typedef void (*fatal_error_print_func)(const char *f, va_list args)
+_X_ATTRIBUTE_PRINTF(1, 0);
+static void
+_FatalError(fatal_error_print_func print_func, const char *f, va_list args)
+_X_ATTRIBUTE_PRINTF(2, 0)
+_X_NORETURN;
+
+void FatalErrorSigSafe(const char *f, ...)
+{
+    va_list args;
+    va_start(args, f);
+    _FatalError(VErrorFSigSafe, f, args);
+    va_end(args);
+}
+
+void FatalError(const char *f, ...)
 {
     va_list args;
+    va_start(args, f);
+    _FatalError(VErrorF, f, args);
+    va_end(args);
+}
+
+static void
+_FatalError(fatal_error_print_func print_func, const char *f, va_list args)
+{
     va_list args2;
     static Bool beenhere = FALSE;
 
     if (beenhere)
-        ErrorF("\nFatalError re-entered, aborting\n");
+        ErrorFSigSafe("\nFatalError re-entered, aborting\n");
     else
-        ErrorF("\nFatal server error:\n");
-
-    va_start(args, f);
+        ErrorFSigSafe("\nFatal server error:\n");
 
     /* Make a copy for OsVendorFatalError */
     va_copy(args2, args);
@@ -800,9 +824,9 @@ FatalError(const char *f, ...)
         va_end(apple_args);
     }
 #endif
-    VErrorF(f, args);
+    (*print_func)(f, args);
     va_end(args);
-    ErrorF("\n");
+    ErrorFSigSafe("\n");
     if (!beenhere)
         OsVendorFatalError(f, args2);
     va_end(args2);
diff --git a/os/osinit.c b/os/osinit.c
index 6c66f9c..94f7393 100644
--- a/os/osinit.c
+++ b/os/osinit.c
@@ -144,8 +144,8 @@ OsSigHandler(int signo)
     }
 #endif
 
-    FatalError("Caught signal %d (%s). Server aborting\n",
-               signo, strsignal(signo));
+    FatalErrorSigSafe("Caught signal %d (%s). Server aborting\n",
+                      signo, strsignal(signo));
 }
 #endif /* !WIN32 || __CYGWIN__ */
 
-- 
1.8.1.2