Fix use after free in fr_syserror and fr_strerror

This ocurred when something called fr_strerror_printf in another thread local destructor, after the memory had been freed.

diff --git a/src/lib/util/strerror.c b/src/lib/util/strerror.c
index 6cb76af0818e3ac759a1119bbc1422cd368a0324..38032082cedb370a4d67bbe68a927d1998d122f1 100644 (file)
--- a/src/lib/util/strerror.c
+++ b/src/lib/util/strerror.c
@@ -49,14 +49,19 @@ typedef struct {
 } fr_log_buffer_t;
 
 fr_thread_local_setup(fr_log_buffer_t *, fr_strerror_buffer)   /* macro */
+static _Thread_local bool logging_stop;        //!< Due to ordering issues we may get errors being
+                                       ///< logged from within other thread local destructors
+                                       ///< which cause a crash on exit if the logging buffer
+                                       ///< has already been freed.
 
 /*
  *     Explicitly cleanup the memory allocated to the error buffer,
  *     just in case valgrind complains about it.
  */
-static void _fr_logging_free(void *arg)
+static void _fr_logging_free(UNUSED void *arg)
 {
-       talloc_free(arg);
+       TALLOC_FREE(fr_strerror_buffer);
+       logging_stop = true;
 }
 
 /** Reset cursor state
@@ -77,6 +82,8 @@ static inline fr_log_buffer_t *fr_strerror_init(void)
 {
        fr_log_buffer_t *buffer;
 
+       if (logging_stop) return NULL;  /* No more logging */
+
        buffer = fr_strerror_buffer;
        if (!buffer) {
                buffer = talloc(NULL, fr_log_buffer_t); /* One byte extra for status */

diff --git a/src/lib/util/syserror.c b/src/lib/util/syserror.c
index cb17c75f8ea903ca7b1b7f8feedaa9834e530750..d409c95bc3cf04865688cb4a2531cc3d70d39fc2 100644 (file)
--- a/src/lib/util/syserror.c
+++ b/src/lib/util/syserror.c
@@ -29,6 +29,12 @@ RCSID("$Id$")
 #define FR_SYSERROR_BUFSIZE (2048)
 
 fr_thread_local_setup(char *, fr_syserror_buffer)              /* macro */
+static _Thread_local bool logging_stop;        //!< Due to ordering issues we may get errors being
+                                       ///< logged from within other thread local destructors
+                                       ///< which cause a crash on exit if the logging buffer
+                                       ///< has already been freed.
+
+#define HAVE_DEFINITION(_errno) ((_errno) < (int)(sizeof(fr_syserror_macro_names) / sizeof(*fr_syserror_macro_names)))
 
 /*
  *     Explicitly cleanup the memory allocated to the error buffer,
@@ -36,7 +42,8 @@ fr_thread_local_setup(char *, fr_syserror_buffer)             /* macro */
  */
 static void _fr_logging_free(UNUSED void *arg)
 {
-       TALLOC_FREE(fr_syserror_buffer);        /* Set to NULL in case the buffer needs to be recreated */
+       TALLOC_FREE(fr_syserror_buffer);
+       logging_stop = true;
 }
 
 /** POSIX-2008 errno macros
@@ -158,6 +165,17 @@ char const *fr_syserror(int num)
 
        buffer = fr_syserror_buffer;
        if (!buffer) {
+               /*
+                *      Try and produce something useful,
+                *      even if the thread is exiting.
+                */
+               if (logging_stop) {
+                       if (HAVE_DEFINITION(num)) {
+                               return fr_syserror_macro_names[num];
+                       }
+                       return "";
+               }
+
                buffer = talloc_array(NULL, char, FR_SYSERROR_BUFSIZE);
                if (!buffer) {
                        fr_perror("Failed allocating memory for system error buffer");
@@ -175,7 +193,7 @@ char const *fr_syserror(int num)
         *      Prefix system errors with the macro name and number
         *      if we're debugging.
         */
-       if (num < (int)(sizeof(fr_syserror_macro_names) / sizeof(*fr_syserror_macro_names))) {
+       if (HAVE_DEFINITION(num)) {
                p += snprintf(p, end - p, "%s: ", fr_syserror_macro_names[num]);
        } else {
                p += snprintf(p, end - p, "errno %i: ", num);