Added checking to protect against simultaneous double free in safemalloc

If two threads would call sf_free() / free_memory() at the same time,
bad_ptr() would not detect this. Fixed by adding extra detection
when working with the memory region under sf_mutex.

Other things:
- If safe_malloc crashes while mutex is hold, stack trace printing will
  hang because we malloc is called by my_open(), which is used by stack
  trace printing code. Fixed by adding MY_NO_REGISTER flag to my_open,
  which will disable the malloc() call to remmeber the file name.

4 files changed, 21 insertions, 6 deletions

diff --git a/include/my_sys.h b/include/my_sys.h
index dcc303730f2..f3515b7726c 100644
--- a/include/my_sys.h
+++ b/include/my_sys.h
@@ -74,6 +74,7 @@ C_MODE_START
 #define MY_SHORT_WAIT	64U	/* my_lock() don't wait if can't lock */
 #define MY_FORCE_LOCK   128U    /* use my_lock() even if disable_locking */
 #define MY_NO_WAIT      256U	/* my_lock() don't wait at all */
+#define MY_NO_REGISTER  8196U   /* my_open(), no malloc for file name */
 /*
   If old_mode is UTF8_IS_UTF8MB3, then pass this flag. It mean utf8 is
   alias for utf8mb3. Otherwise utf8 is alias for utf8mb4.
diff --git a/mysys/my_open.c b/mysys/my_open.c
index 8b5f4f9435e..4d26a7b250e 100644
--- a/mysys/my_open.c
+++ b/mysys/my_open.c
@@ -135,7 +135,7 @@ File my_register_filename(File fd, const char *FileName, enum file_type
   if ((int) fd >= MY_FILE_MIN)
   {
     my_atomic_add32_explicit(&my_file_opened, 1, MY_MEMORY_ORDER_RELAXED);
-    if ((uint) fd >= my_file_limit)
+    if ((uint) fd >= my_file_limit || (MyFlags & MY_NO_REGISTER))
       DBUG_RETURN(fd);
     my_file_info[fd].name = my_strdup(key_memory_my_file_info, FileName, MyFlags);
     statistic_increment(my_file_total_opened,&THR_LOCK_open);
diff --git a/mysys/safemalloc.c b/mysys/safemalloc.c
index 34e57f36bb5..57ffd28a336 100644
--- a/mysys/safemalloc.c
+++ b/mysys/safemalloc.c
@@ -78,6 +78,7 @@ static void  *sf_min_adress= (void*) (intptr)~0ULL,
 static struct st_irem *sf_malloc_root = 0;
 
 #define MAGICSTART      0x14235296      /* A magic value for underrun key */
+#define MAGICEND        0x12345678      /* Value for freed block */
 
 #define MAGICEND0       0x68            /* Magic values for overrun keys  */
 #define MAGICEND1       0x34            /*              "                 */
@@ -255,6 +256,7 @@ static void print_stack(void **frame)
 static void free_memory(void *ptr)
 {
   struct st_irem *irem= (struct st_irem *)ptr - 1;
+  size_t end_offset;
 
   if ((irem->flags & MY_THREAD_SPECIFIC) && irem->thread_id &&
       irem->thread_id != sf_malloc_dbug_id())
@@ -266,6 +268,14 @@ static void free_memory(void *ptr)
   }
 
   pthread_mutex_lock(&sf_mutex);
+  /* Protect against double free at same time */
+  if (irem->marker != MAGICSTART)
+  {
+    pthread_mutex_unlock(&sf_mutex);            /* Allow stack trace alloc mem */
+    DBUG_ASSERT(irem->marker == MAGICSTART);    /* Crash */
+    pthread_mutex_lock(&sf_mutex);              /* Impossible, but safer */
+  }
+
   /* Remove this structure from the linked list */
   if (irem->prev)
     irem->prev->next= irem->next;
@@ -277,10 +287,13 @@ static void free_memory(void *ptr)
 
   /* Handle the statistics */
   sf_malloc_count--;
+
+  irem->marker= MAGICEND;                       /* Double free detection */
   pthread_mutex_unlock(&sf_mutex);
 
-  /* only trash the data and magic values, but keep the stack trace */
-  TRASH_FREE((uchar*)(irem + 1) - 4, irem->datasize + 8);
+  /* Trash the data and magic values, but keep the stack trace */
+  end_offset= sizeof(*irem) - ((char*) &irem->marker - (char*) irem);
+  TRASH_FREE((uchar*)(irem + 1) - end_offset, irem->datasize + 4 + end_offset);
   free(irem);
   return;
 }
diff --git a/sql/signal_handler.cc b/sql/signal_handler.cc
index 6aac9ac5962..7b3cfefa377 100644
--- a/sql/signal_handler.cc
+++ b/sql/signal_handler.cc
@@ -65,9 +65,9 @@ static inline void output_core_info()
                           (int) len, buff);
   }
 #ifdef __FreeBSD__
-  if ((fd= my_open("/proc/curproc/rlimit", O_RDONLY, MYF(0))) >= 0)
+  if ((fd= my_open("/proc/curproc/rlimit", O_RDONLY, MYF(MY_NO_REGISTER))) >= 0)
 #else
-  if ((fd= my_open("/proc/self/limits", O_RDONLY, MYF(0))) >= 0)
+  if ((fd= my_open("/proc/self/limits", O_RDONLY, MYF(MY_NO_REGISTER))) >= 0)
 #endif
   {
     my_safe_printf_stderr("Resource Limits:\n");
@@ -78,7 +78,8 @@ static inline void output_core_info()
     my_close(fd, MYF(0));
   }
 #ifdef __linux__
-  if ((fd= my_open("/proc/sys/kernel/core_pattern", O_RDONLY, MYF(0))) >= 0)
+  if ((fd= my_open("/proc/sys/kernel/core_pattern", O_RDONLY,
+                   MYF(MY_NO_REGISTER))) >= 0)
   {
     len= my_read(fd, (uchar*)buff, sizeof(buff),  MYF(0));
     my_safe_printf_stderr("Core pattern: %.*s\n", (int) len, buff);