Merge pull request #2719 from matt335672/log_xrdp_termination_signal

Log xrdp termination signals

common/string_calls.c

@@ -21,6 +21,7 @@
 #if defined(HAVE_CONFIG_H)
 #include "config_ac.h"
 #endif
+#include <signal.h>
 #include <string.h>
 #include <strings.h>
 #include <stdlib.h>
@@ -1143,3 +1144,147 @@ g_charstr_to_bitmask(const char *str, const struct bitmask_char bitdefs[],
     return bitmask;
 }
 
+/*****************************************************************************/
+/*
+ * Looks for a simple mapping of signal number to name
+ */
+static const char *
+find_sig_name(int signum)
+{
+    typedef struct
+    {
+        int num;
+        const char *name;
+    } sig_to_name_type;
+
+    // Map a string 'zzz' to { SIGzzz, "zzz"} for making
+    // typo-free sig_to_name_type objects
+#   define DEFSIG(sig) { SIG ## sig, # sig }
+
+    // Entries in this array are taken from
+    // The Single UNIX ® Specification, Version 2 (1997)
+    // plus additions from specific operating systems.
+    //
+    // The SUS requires these to be positive integer constants with a
+    // macro definition.  Note that SIGRTMIN and SIGRTMAX on Linux are
+    // NOT constants, so have to be handled separately.
+    static const sig_to_name_type sigmap[] =
+    {
+        // Names from SUS v2, in the order they are listed in that document
+        // that *should* be defined everywhere
+        //
+        // Commented out definitions below are NOT used everywhere
+        DEFSIG(ABRT), DEFSIG(ALRM), DEFSIG(FPE), DEFSIG(HUP),
+        DEFSIG(ILL), DEFSIG(INT), DEFSIG(KILL), DEFSIG(PIPE),
+        DEFSIG(QUIT), DEFSIG(SEGV), DEFSIG(TERM), DEFSIG(USR1),
+        DEFSIG(USR2), DEFSIG(CHLD), DEFSIG(CONT), DEFSIG(STOP),
+        DEFSIG(TSTP), DEFSIG(TTIN), DEFSIG(TTOU), DEFSIG(BUS),
+        /* DEFSIG(POLL), */ /* DEFSIG(PROF), */ DEFSIG(SYS), DEFSIG(TRAP),
+        DEFSIG(URG), DEFSIG(VTALRM), DEFSIG(XCPU), DEFSIG(XFSZ),
+
+        // SIGPOLL and SIGPROF are marked as obselescent in 1003.1-2017,
+        // Also SIGPOLL isn't in *BSD operating systems which use SIGIO
+#ifdef SIGPOLL
+        DEFSIG(POLL),
+#endif
+#ifdef SIGPROF
+        DEFSIG(PROF),
+#endif
+
+        // BSD signals (from FreeBSD/OpenBSD sys/signal.h and
+        // Darwin/Illumos signal.h)
+#ifdef SIGEMT
+        DEFSIG(EMT),
+#endif
+#ifdef SIGIO
+        DEFSIG(IO),
+#endif
+#ifdef SIGWINCH
+        DEFSIG(WINCH),
+#endif
+#ifdef SIGINFO
+        DEFSIG(INFO),
+#endif
+#ifdef SIGTHR
+        DEFSIG(THR),
+#endif
+#ifdef SIGLIBRT
+        DEFSIG(LIBRT),
+#endif
+#ifdef SIGPWR
+        DEFSIG(PWR),
+#endif
+#ifdef SIGWAITING
+        DEFSIG(WAITING),
+#endif
+#ifdef SIGLWP
+        DEFSIG(LWP),
+#endif
+
+        // Linux additions to *BSD (signal(7))
+#ifdef SIGLOST
+        DEFSIG(LOST),
+#endif
+#ifdef SIGSTKFLT
+        DEFSIG(STKFLT),
+#endif
+
+        // Terminator
+        {0, NULL}
+#undef DEFSIG
+    };
+
+    const sig_to_name_type *p;
+
+    for (p = &sigmap[0] ; p->name != NULL ; ++p)
+    {
+        if (p->num == signum)
+        {
+            return p->name;
+        }
+    }
+
+    // These aren't constants on Linux
+#ifdef SIGRTMIN
+    if (signum == SIGRTMIN)
+    {
+        return "RTMIN";
+    }
+#endif
+#ifdef SIGRTMAX
+    if (signum == SIGRTMAX)
+    {
+        return "RTMAX";
+    }
+#endif
+
+    return NULL;
+}
+
+/*****************************************************************************/
+char *
+g_sig2text(int signum, char sigstr[])
+{
+    if (signum >= 0)
+    {
+        const char *name = find_sig_name(signum);
+
+        if (name != NULL)
+        {
+            g_snprintf(sigstr, MAXSTRSIGLEN, "SIG%s", name);
+            return sigstr;
+        }
+
+#if defined(SIGRTMIN) && defined(SIGRTMAX)
+        if (signum > SIGRTMIN && signum < SIGRTMAX)
+        {
+            g_snprintf(sigstr, MAXSTRSIGLEN, "SIGRTMIN+%d", signum - SIGRTMIN);
+            return sigstr;
+        }
+#endif
+    }
+
+    // If all else fails...
+    g_snprintf(sigstr, MAXSTRSIGLEN, "SIG#%d", signum);
+    return sigstr;
+}

common/string_calls.h

@@ -67,6 +67,26 @@ struct bitmask_char
 
 #define BITMASK_CHAR_END_OF_LIST { 0, '\0' }
 
+enum
+{
+    // See g_sig2text()
+    // Must be able to hold "SIG#%d" for INT_MIN
+    //
+    // ((sizeof(int) * 5 + 1) / 2) provides a very slight overestimate of
+    // the bytes requires to store a decimal expansion of 'int':-
+    // sizeof  INT_MAX     display bytes   ((sizeof(int) * 5 + 1)
+    // (int)               needed           / 2)
+    // ------  -------     -------------   ---------------------------
+    // 1       127         3                3
+    // 2       32767       5                5
+    // 3       8388607     7                8
+    // 4       2147483637  10              10
+    // 8       9*(10**18)  19              20
+    // 16      2*(10**38)  39              40
+    // 32      6*(10**76)  77              80
+    MAXSTRSIGLEN =  (3 + 1 + 1 + ((sizeof(int) * 5 + 1) / 2) + 1)
+};
+
 /**
  * Processes a format string for general info
  *
@@ -266,6 +286,7 @@ int      g_strncmp_d(const char *c1, const char *c2, const char delim, int len);
 int      g_strcasecmp(const char *c1, const char *c2);
 int      g_strncasecmp(const char *c1, const char *c2, int len);
 int      g_atoi(const char *str);
+
 /**
  * Extends g_atoi(), Converts decimal and hexadecimal number String to integer
  *
@@ -283,4 +304,17 @@ char    *g_strstr(const char *haystack, const char *needle);
 int      g_mbstowcs(twchar *dest, const char *src, int n);
 int      g_wcstombs(char *dest, const twchar *src, int n);
 int      g_strtrim(char *str, int trim_flags);
+
+/**
+ * Maps a signal number to a string, i.e. SIGHUP -> "SIGHUP"
+ *
+ * @param signum Signal number
+ * @param sigstr buffer for result
+ * @return sigstr, for convenience
+ *
+ * Buffer is assumed to be at least MAXSTRSIGLEN
+ *
+ * The string "SIG#<num>" is returned for unrecognised signums
+ */
+char    *g_sig2text(int signum, char sigstr[]);
 #endif

sesman/sesexec/session.c

@@ -821,8 +821,12 @@ exit_status_to_str(const struct exit_status *e, char buff[], int bufflen)
             break;
 
         case E_XR_SIGNAL:
-            g_snprintf(buff, bufflen, "signal %d", e->val);
-            break;
+        {
+            char sigstr[MAXSTRSIGLEN];
+            g_snprintf(buff, bufflen, "signal %s",
+                       g_sig2text(e->val, sigstr));
+        }
+        break;
 
         default:
             g_snprintf(buff, bufflen, "an unexpected error");

sesman/sesexec/xwait.c

@@ -155,10 +155,13 @@ wait_for_xserver(uid_t uid,
                         break;
 
                     case E_XR_SIGNAL:
+                    {
+                        char sigstr[MAXSTRSIGLEN];
                         LOG(LOG_LEVEL_ERROR,
-                            "waitforx failed with unexpected signal %d",
-                            e.val);
-                        break;
+                            "waitforx failed with unexpected signal %s",
+                            g_sig2text(e.val, sigstr));
+                    }
+                    break;
 
                     default:
                         LOG(LOG_LEVEL_ERROR,

tests/common/test_string_calls.c

@@ -3,6 +3,10 @@
 #include "config_ac.h"
 #endif
 
+#include <limits.h>
+#include <signal.h>
+
+#include "os_calls.h"
 #include "string_calls.h"
 #include "ms-rdpbcgr.h"
 
@@ -1036,6 +1040,76 @@ END_TEST
 
 /******************************************************************************/
 
+START_TEST(test_sigs__common)
+{
+    char name[MAXSTRSIGLEN];
+    char *res;
+
+    // Check some common POSIX signals
+    res = g_sig2text(SIGHUP, name);
+    ck_assert_ptr_eq(res, name);
+    ck_assert_str_eq(res, "SIGHUP");
+
+    res = g_sig2text(SIGCHLD, name);
+    ck_assert_ptr_eq(res, name);
+    ck_assert_str_eq(res, "SIGCHLD");
+
+    res = g_sig2text(SIGXFSZ, name);
+    ck_assert_ptr_eq(res, name);
+    ck_assert_str_eq(res, "SIGXFSZ");
+
+    res = g_sig2text(SIGRTMIN, name);
+    ck_assert_ptr_eq(res, name);
+    ck_assert_str_eq(res, "SIGRTMIN");
+
+    res = g_sig2text(SIGRTMIN + 2, name);
+    ck_assert_ptr_eq(res, name);
+    ck_assert_str_eq(res, "SIGRTMIN+2");
+
+    res = g_sig2text(SIGRTMAX, name);
+    ck_assert_ptr_eq(res, name);
+    ck_assert_str_eq(res, "SIGRTMAX");
+
+    // Should be invalid
+    res = g_sig2text(0, name);
+    ck_assert_ptr_eq(res, name);
+    ck_assert_str_eq(res, "SIG#0");
+
+    res = g_sig2text(65535, name);
+    ck_assert_ptr_eq(res, name);
+    ck_assert_str_eq(res, "SIG#65535");
+
+
+    // POSIX defines signals as ints, but insists they are positive. So
+    // we ought to trest we get sane behaviour for -ve numbers
+    res = g_sig2text(-1, name);
+    ck_assert_ptr_eq(res, name);
+    ck_assert_str_eq(res, "SIG#-1");
+}
+END_TEST
+
+START_TEST(test_sigs__bigint)
+{
+    char name[MAXSTRSIGLEN];
+    char name2[1024];
+
+    // Check that big integers aren't being truncated by the definition
+    // of MAXSTRSIGLEN
+    int i = INT_MAX;
+
+    g_sig2text(i, name);
+    g_snprintf(name2, sizeof(name2), "SIG#%d", i);
+    ck_assert_str_eq(name, name2);
+
+    i = INT_MIN;
+    g_sig2text(i, name);
+    g_snprintf(name2, sizeof(name2), "SIG#%d", i);
+    ck_assert_str_eq(name, name2);
+}
+END_TEST
+
+/******************************************************************************/
+
 Suite *
 make_suite_test_string(void)
 {
@@ -1046,6 +1120,7 @@ make_suite_test_string(void)
     TCase *tc_bm2char;
     TCase *tc_char2bm;
     TCase *tc_strtrim;
+    TCase *tc_sigs;
 
     s = suite_create("String");
 
@@ -1118,5 +1193,10 @@ make_suite_test_string(void)
     tcase_add_test(tc_strtrim, test_strtrim__trim_both);
     tcase_add_test(tc_strtrim, test_strtrim__trim_through);
 
+    tc_sigs = tcase_create("signals");
+    suite_add_tcase(s, tc_sigs);
+    tcase_add_test(tc_sigs, test_sigs__common);
+    tcase_add_test(tc_sigs, test_sigs__bigint);
+
     return s;
 }

xrdp/xrdp.c

@@ -102,9 +102,7 @@ xrdp_shutdown(int sig)
 static void
 xrdp_child(int sig)
 {
-    while (g_waitchild(NULL) > 0)
-    {
-    }
+    g_set_sigchld(1);
 }
 
 /*****************************************************************************/
@@ -562,6 +560,14 @@ main(int argc, char **argv)
         LOG(LOG_LEVEL_WARNING, "error creating g_term_event");
     }
 
+    g_snprintf(text, 255, "xrdp_%8.8x_main_sigchld", pid);
+    g_set_sigchld_event(g_create_wait_obj(text));
+
+    if (g_get_sigchld() == 0)
+    {
+        LOG(LOG_LEVEL_WARNING, "error creating g_sigchld_event");
+    }
+
     g_snprintf(text, 255, "xrdp_%8.8x_main_sync", pid);
     g_set_sync_event(g_create_wait_obj(text));
 
@@ -583,6 +589,9 @@ main(int argc, char **argv)
     g_delete_wait_obj(g_get_term());
     g_set_term_event(0);
 
+    g_delete_wait_obj(g_get_sigchld());
+    g_set_sigchld_event(0);
+
     g_delete_wait_obj(g_get_sync_event());
     g_set_sync_event(0);