1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Added checking to protect against simultaneous double free in safemalloc

Browse Source 
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.
This commit is contained in:
Monty 2021-04-30 17:08:11 +03:00 committed by Sergei Golubchik
parent 744a53806e
commit 79d9a725f9
4 changed files with 21 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/my_sys.h
View File

@ -74,6 +74,7 @@ C_MODE_START
#define MY_SHORT_WAIT 64U /* my_lock() don't wait if can't lock */ #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_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_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 If old_mode is UTF8_IS_UTF8MB3, then pass this flag. It mean utf8 is
alias for utf8mb3. Otherwise utf8 is alias for utf8mb4. alias for utf8mb3. Otherwise utf8 is alias for utf8mb4.

2
mysys/my_open.c
View File

@ -135,7 +135,7 @@ File my_register_filename(File fd, const char *FileName, enum file_type
if ((int) fd >= MY_FILE_MIN) if ((int) fd >= MY_FILE_MIN)
{ {
my_atomic_add32_explicit(&my_file_opened, 1, MY_MEMORY_ORDER_RELAXED); 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); DBUG_RETURN(fd);
my_file_info[fd].name = my_strdup(key_memory_my_file_info, FileName, MyFlags); my_file_info[fd].name = my_strdup(key_memory_my_file_info, FileName, MyFlags);
statistic_increment(my_file_total_opened,&THR_LOCK_open); statistic_increment(my_file_total_opened,&THR_LOCK_open);

17
mysys/safemalloc.c
View File

@ -78,6 +78,7 @@ static void *sf_min_adress= (void*) (intptr)~0ULL,
static struct st_irem *sf_malloc_root = 0; static struct st_irem *sf_malloc_root = 0;
#define MAGICSTART 0x14235296 /* A magic value for underrun key */ #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 MAGICEND0 0x68 /* Magic values for overrun keys */
#define MAGICEND1 0x34 /* " */ #define MAGICEND1 0x34 /* " */
@ -255,6 +256,7 @@ static void print_stack(void **frame)
static void free_memory(void *ptr) static void free_memory(void *ptr)
{ {
struct st_irem *irem= (struct st_irem *)ptr - 1; struct st_irem *irem= (struct st_irem *)ptr - 1;
size_t end_offset;
if ((irem->flags & MY_THREAD_SPECIFIC) && irem->thread_id && if ((irem->flags & MY_THREAD_SPECIFIC) && irem->thread_id &&
irem->thread_id != sf_malloc_dbug_id()) irem->thread_id != sf_malloc_dbug_id())
@ -266,6 +268,14 @@ static void free_memory(void *ptr)
} }
pthread_mutex_lock(&sf_mutex); 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 */ /* Remove this structure from the linked list */
if (irem->prev) if (irem->prev)
irem->prev->next= irem->next; irem->prev->next= irem->next;
@ -277,10 +287,13 @@ static void free_memory(void *ptr)
/* Handle the statistics */ /* Handle the statistics */
sf_malloc_count--; sf_malloc_count--;
irem->marker= MAGICEND; /* Double free detection */
pthread_mutex_unlock(&sf_mutex); pthread_mutex_unlock(&sf_mutex);
/* only trash the data and magic values, but keep the stack trace */ /* Trash the data and magic values, but keep the stack trace */
TRASH_FREE((uchar*)(irem + 1) - 4, irem->datasize + 8); end_offset= sizeof(*irem) - ((char*) &irem->marker - (char*) irem);
TRASH_FREE((uchar*)(irem + 1) - end_offset, irem->datasize + 4 + end_offset);
free(irem); free(irem);
return; return;
} }

7
sql/signal_handler.cc
View File

@ -65,9 +65,9 @@ static inline void output_core_info()
(int) len, buff); (int) len, buff);
} }
#ifdef __FreeBSD__ #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 #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 #endif
{ {
my_safe_printf_stderr("Resource Limits:\n"); my_safe_printf_stderr("Resource Limits:\n");
@ -78,7 +78,8 @@ static inline void output_core_info()
my_close(fd, MYF(0)); my_close(fd, MYF(0));
} }
#ifdef __linux__ #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)); len= my_read(fd, (uchar*)buff, sizeof(buff), MYF(0));
my_safe_printf_stderr("Core pattern: %.*s\n", (int) len, buff); my_safe_printf_stderr("Core pattern: %.*s\n", (int) len, buff);