[asterisk-commits] rmudgett: branch 11 r397528 - in /branches/11: ./ channels/ include/asterisk/...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Aug 23 11:07:22 CDT 2013


Author: rmudgett
Date: Fri Aug 23 11:07:18 2013
New Revision: 397528

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=397528
Log:
Fix memory corruption when trying to get "core show locks".

Review https://reviewboard.asterisk.org/r/2580/ tried to fix the mismatch
in memory pools but had a math error determining the buffer size and
didn't address other similar memory pool mismatches.

* Effectively reverted the previous patch to go in the same direction as
trunk for the returned memory pool of ast_bt_get_symbols().

* Fixed memory leak in ast_bt_get_symbols() when BETTER_BACKTRACES is
defined.

* Fixed some formatting in ast_bt_get_symbols().

* Fixed sig_pri.c freeing memory allocated by libpri when MALLOC_DEBUG is
enabled.

* Fixed __dump_backtrace() freeing memory from ast_bt_get_symbols() when
MALLOC_DEBUG is enabled.

* Moved __dump_backtrace() because of compile issues with the utils
directory.

(closes issue ASTERISK-22221)
Reported by: Matt Jordan

Review: https://reviewboard.asterisk.org/r/2778/
........

Merged revisions 397525 from http://svn.asterisk.org/svn/asterisk/branches/1.8

Modified:
    branches/11/   (props changed)
    branches/11/channels/sig_pri.c
    branches/11/include/asterisk/astmm.h
    branches/11/include/asterisk/lock.h
    branches/11/include/asterisk/logger.h
    branches/11/include/asterisk/utils.h
    branches/11/main/astmm.c
    branches/11/main/astobj2.c
    branches/11/main/lock.c
    branches/11/main/logger.c
    branches/11/main/utils.c

Propchange: branches/11/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.

Modified: branches/11/channels/sig_pri.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/channels/sig_pri.c?view=diff&rev=397528&r1=397527&r2=397528
==============================================================================
--- branches/11/channels/sig_pri.c (original)
+++ branches/11/channels/sig_pri.c Fri Aug 23 11:07:18 2013
@@ -9286,7 +9286,7 @@
 			info_str = pri_dump_info_str(pri->pri);
 			if (info_str) {
 				ast_cli(fd, "%s", info_str);
-				free(info_str);
+				ast_std_free(info_str);
 			}
 #else
 			pri_dump_info(pri->pri);

Modified: branches/11/include/asterisk/astmm.h
URL: http://svnview.digium.com/svn/asterisk/branches/11/include/asterisk/astmm.h?view=diff&rev=397528&r1=397527&r2=397528
==============================================================================
--- branches/11/include/asterisk/astmm.h (original)
+++ branches/11/include/asterisk/astmm.h Fri Aug 23 11:07:18 2013
@@ -53,6 +53,12 @@
 #undef asprintf
 #undef vasprintf
 #undef free
+
+void *ast_std_malloc(size_t size);
+void *ast_std_calloc(size_t nmemb, size_t size);
+void *ast_std_realloc(void *ptr, size_t size);
+void ast_std_free(void *ptr);
+void ast_free_ptr(void *ptr);
 
 void *__ast_calloc(size_t nmemb, size_t size, const char *file, int lineno, const char *func);
 void *__ast_calloc_cache(size_t nmemb, size_t size, const char *file, int lineno, const char *func);

Modified: branches/11/include/asterisk/lock.h
URL: http://svnview.digium.com/svn/asterisk/branches/11/include/asterisk/lock.h?view=diff&rev=397528&r1=397527&r2=397528
==============================================================================
--- branches/11/include/asterisk/lock.h (original)
+++ branches/11/include/asterisk/lock.h Fri Aug 23 11:07:18 2013
@@ -288,22 +288,6 @@
 #define ast_remove_lock_info(ignore)
 #endif /* HAVE_BKTR */
 #endif /* !defined(LOW_MEMORY) */
-
-#ifdef HAVE_BKTR
-static inline void __dump_backtrace(struct ast_bt *bt, int canlog)
-{
-	char **strings;
-
-	ssize_t i;
-
-	strings = backtrace_symbols(bt->addresses, bt->num_frames);
-
-	for (i = 0; i < bt->num_frames; i++)
-		__ast_mutex_logger("%s\n", strings[i]);
-
-	free(strings);
-}
-#endif
 
 /*!
  * \brief log info for the current lock with ast_log().

Modified: branches/11/include/asterisk/logger.h
URL: http://svnview.digium.com/svn/asterisk/branches/11/include/asterisk/logger.h?view=diff&rev=397528&r1=397527&r2=397528
==============================================================================
--- branches/11/include/asterisk/logger.h (original)
+++ branches/11/include/asterisk/logger.h Fri Aug 23 11:07:18 2013
@@ -411,7 +411,7 @@
  * \param addresses A list of addresses, such as the ->addresses structure element of struct ast_bt.
  * \param num_frames Number of addresses in the addresses list
  * \retval NULL Unable to allocate memory
- * \return List of strings. Free the entire list with a single ast_free call.
+ * \return List of strings. Free the entire list with a single ast_std_free call.
  * \since 1.6.2.16
  */
 char **ast_bt_get_symbols(void **addresses, size_t num_frames);

Modified: branches/11/include/asterisk/utils.h
URL: http://svnview.digium.com/svn/asterisk/branches/11/include/asterisk/utils.h?view=diff&rev=397528&r1=397527&r2=397528
==============================================================================
--- branches/11/include/asterisk/utils.h (original)
+++ branches/11/include/asterisk/utils.h Fri Aug 23 11:07:18 2013
@@ -459,24 +459,20 @@
 long int ast_random(void);
 
 
+#ifndef __AST_DEBUG_MALLOC
+#define ast_std_malloc malloc
+#define ast_std_calloc calloc
+#define ast_std_realloc realloc
+#define ast_std_free free
+
 /*!
  * \brief free() wrapper
  *
  * ast_free_ptr should be used when a function pointer for free() needs to be passed
  * as the argument to a function. Otherwise, astmm will cause seg faults.
  */
-#ifdef __AST_DEBUG_MALLOC
-static void ast_free_ptr(void *ptr) attribute_unused;
-static void ast_free_ptr(void *ptr)
-{
-	ast_free(ptr);
-}
-#else
 #define ast_free free
 #define ast_free_ptr ast_free
-#endif
-
-#ifndef __AST_DEBUG_MALLOC
 
 #define MALLOC_FAILURE_MSG \
 	ast_log(LOG_ERROR, "Memory Allocation Failure in function %s at line %d of %s\n", func, lineno, file);

Modified: branches/11/main/astmm.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/main/astmm.c?view=diff&rev=397528&r1=397527&r2=397528
==============================================================================
--- branches/11/main/astmm.c (original)
+++ branches/11/main/astmm.c Fri Aug 23 11:07:18 2013
@@ -156,6 +156,31 @@
 			fflush(mmlog);               \
 		}                                    \
 	} while (0)
+
+void *ast_std_malloc(size_t size)
+{
+	return malloc(size);
+}
+
+void *ast_std_calloc(size_t nmemb, size_t size)
+{
+	return calloc(nmemb, size);
+}
+
+void *ast_std_realloc(void *ptr, size_t size)
+{
+	return realloc(ptr, size);
+}
+
+void ast_std_free(void *ptr)
+{
+	free(ptr);
+}
+
+void ast_free_ptr(void *ptr)
+{
+	ast_free(ptr);
+}
 
 /*!
  * \internal

Modified: branches/11/main/astobj2.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/main/astobj2.c?view=diff&rev=397528&r1=397527&r2=397528
==============================================================================
--- branches/11/main/astobj2.c (original)
+++ branches/11/main/astobj2.c Fri Aug 23 11:07:18 2013
@@ -123,7 +123,7 @@
 	for(i = 0; i < c; i++) {
 		ast_verbose("%d: %p %s\n", i, addresses[i], strings[i]);
 	}
-	ast_free(strings);
+	ast_std_free(strings);
 }
 #endif
 

Modified: branches/11/main/lock.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/main/lock.c?view=diff&rev=397528&r1=397527&r2=397528
==============================================================================
--- branches/11/main/lock.c (original)
+++ branches/11/main/lock.c Fri Aug 23 11:07:18 2013
@@ -29,6 +29,7 @@
 
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
+#include "asterisk/utils.h"
 #include "asterisk/lock.h"
 
 /* Allow direct use of pthread_mutex_* / pthread_cond_* */
@@ -44,6 +45,22 @@
 #undef pthread_cond_destroy
 #undef pthread_cond_wait
 #undef pthread_cond_timedwait
+
+#if defined(DEBUG_THREADS) && defined(HAVE_BKTR)
+static void __dump_backtrace(struct ast_bt *bt, int canlog)
+{
+	char **strings;
+	ssize_t i;
+
+	strings = backtrace_symbols(bt->addresses, bt->num_frames);
+
+	for (i = 0; i < bt->num_frames; i++) {
+		__ast_mutex_logger("%s\n", strings[i]);
+	}
+
+	ast_std_free(strings);
+}
+#endif	/* defined(DEBUG_THREADS) && defined(HAVE_BKTR) */
 
 int __ast_pthread_mutex_init(int tracking, const char *filename, int lineno, const char *func,
 						const char *mutex_name, ast_mutex_t *t)

Modified: branches/11/main/logger.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/main/logger.c?view=diff&rev=397528&r1=397527&r2=397528
==============================================================================
--- branches/11/main/logger.c (original)
+++ branches/11/main/logger.c Fri Aug 23 11:07:18 2013
@@ -1593,7 +1593,7 @@
 
 char **ast_bt_get_symbols(void **addresses, size_t num_frames)
 {
-	char **strings = NULL;
+	char **strings;
 #if defined(BETTER_BACKTRACES)
 	int stackfr;
 	bfd *bfdobj;           /* bfd.h */
@@ -1613,9 +1613,12 @@
 
 #if defined(BETTER_BACKTRACES)
 	strings_size = num_frames * sizeof(*strings);
+
 	eachlen = ast_calloc(num_frames, sizeof(*eachlen));
-
-	if (!(strings = ast_calloc(num_frames, sizeof(*strings)))) {
+	strings = ast_std_calloc(num_frames, sizeof(*strings));
+	if (!eachlen || !strings) {
+		ast_free(eachlen);
+		ast_std_free(strings);
 		return NULL;
 	}
 
@@ -1630,6 +1633,7 @@
 
 		if (strcmp(dli.dli_fname, "asterisk") == 0) {
 			char asteriskpath[256];
+
 			if (!(dli.dli_fname = ast_utils_which("asterisk", asteriskpath, sizeof(asteriskpath)))) {
 				/* This will fail to find symbols */
 				ast_debug(1, "Failed to find asterisk binary for debug symbols.\n");
@@ -1638,11 +1642,11 @@
 		}
 
 		lastslash = strrchr(dli.dli_fname, '/');
-		if (	(bfdobj = bfd_openr(dli.dli_fname, NULL)) &&
-				bfd_check_format(bfdobj, bfd_object) &&
-				(allocsize = bfd_get_symtab_upper_bound(bfdobj)) > 0 &&
-				(syms = ast_malloc(allocsize)) &&
-				(symbolcount = bfd_canonicalize_symtab(bfdobj, syms))) {
+		if ((bfdobj = bfd_openr(dli.dli_fname, NULL)) &&
+			bfd_check_format(bfdobj, bfd_object) &&
+			(allocsize = bfd_get_symtab_upper_bound(bfdobj)) > 0 &&
+			(syms = ast_malloc(allocsize)) &&
+			(symbolcount = bfd_canonicalize_symtab(bfdobj, syms))) {
 
 			if (bfdobj->flags & DYNAMIC) {
 				offset = addresses[stackfr] - dli.dli_fbase;
@@ -1651,9 +1655,9 @@
 			}
 
 			for (section = bfdobj->sections; section; section = section->next) {
-				if (	!bfd_get_section_flags(bfdobj, section) & SEC_ALLOC ||
-						section->vma > offset ||
-						section->size + section->vma < offset) {
+				if (!bfd_get_section_flags(bfdobj, section) & SEC_ALLOC ||
+					section->vma > offset ||
+					section->size + section->vma < offset) {
 					continue;
 				}
 
@@ -1668,7 +1672,9 @@
 				found++;
 				if ((lastslash = strrchr(file, '/'))) {
 					const char *prevslash;
-					for (prevslash = lastslash - 1; *prevslash != '/' && prevslash >= file; prevslash--);
+
+					for (prevslash = lastslash - 1; *prevslash != '/' && prevslash >= file; prevslash--) {
+					}
 					if (prevslash >= file) {
 						lastslash = prevslash;
 					}
@@ -1690,9 +1696,7 @@
 		}
 		if (bfdobj) {
 			bfd_close(bfdobj);
-			if (syms) {
-				ast_free(syms);
-			}
+			ast_free(syms);
 		}
 
 		/* Default output, if we cannot find the information within BFD */
@@ -1712,52 +1716,32 @@
 
 		if (!ast_strlen_zero(msg)) {
 			char **tmp;
-			eachlen[stackfr] = strlen(msg);
-			if (!(tmp = ast_realloc(strings, strings_size + eachlen[stackfr] + 1))) {
-				ast_free(strings);
+
+			eachlen[stackfr] = strlen(msg) + 1;
+			if (!(tmp = ast_std_realloc(strings, strings_size + eachlen[stackfr]))) {
+				ast_std_free(strings);
 				strings = NULL;
 				break; /* out of stack frame iteration */
 			}
 			strings = tmp;
 			strings[stackfr] = (char *) strings + strings_size;
-			ast_copy_string(strings[stackfr], msg, eachlen[stackfr] + 1);
-			strings_size += eachlen[stackfr] + 1;
+			strcpy(strings[stackfr], msg);/* Safe since we just allocated the room. */
+			strings_size += eachlen[stackfr];
 		}
 	}
 
 	if (strings) {
-		/* Recalculate the offset pointers */
+		/* Recalculate the offset pointers because of the reallocs. */
 		strings[0] = (char *) strings + num_frames * sizeof(*strings);
 		for (stackfr = 1; stackfr < num_frames; stackfr++) {
-			strings[stackfr] = strings[stackfr - 1] + eachlen[stackfr - 1] + 1;
-		}
-	}
+			strings[stackfr] = strings[stackfr - 1] + eachlen[stackfr - 1];
+		}
+	}
+	ast_free(eachlen);
+
 #else /* !defined(BETTER_BACKTRACES) */
-	if ((strings = backtrace_symbols(addresses, num_frames))) {
-		/* Re-do value into ast_alloc'ed memory */
-		char **ast_strings;
-		char *p;
-		unsigned size = num_frames + sizeof(*strings);
-		int i;
-		for (i = 0; i < num_frames; ++i) {
-			size += strlen(strings[i]) + 1;
-		}
-#undef free
-		if (!(ast_strings = ast_malloc(size))) {
-			free(strings);
-			ast_log(LOG_WARNING, "Unable to re-allocate space for backtrace structure\n");
-			return NULL;
-		}
-		p = (char *) (ast_strings + num_frames);
-		for (i = 0; i < num_frames; ++i) {
-			unsigned len = strlen(strings[i]) + 1;
-			ast_strings[i] = p;
-			memcpy(p, strings[i], len);
-			p += len;
-		}
-		free(strings);
-		strings = ast_strings;
-	}
+
+	strings = backtrace_symbols(addresses, num_frames);
 #endif /* defined(BETTER_BACKTRACES) */
 	return strings;
 }
@@ -1782,7 +1766,7 @@
 			ast_debug(1, "#%d: [%p] %s\n", i - 3, bt->addresses[i], strings[i]);
 		}
 
-		ast_free(strings);
+		ast_std_free(strings);
 	} else {
 		ast_debug(1, "Could not allocate memory for backtrace\n");
 	}

Modified: branches/11/main/utils.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/main/utils.c?view=diff&rev=397528&r1=397527&r2=397528
==============================================================================
--- branches/11/main/utils.c (original)
+++ branches/11/main/utils.c Fri Aug 23 11:07:18 2013
@@ -869,7 +869,7 @@
 			ast_str_append(str, 0, "\t%s\n", symbols[frame_iterator]);
 		}
 
-		ast_free(symbols);
+		ast_std_free(symbols);
 	} else {
 		ast_str_append(str, 0, "\tCouldn't retrieve backtrace symbols\n");
 	}




More information about the asterisk-commits mailing list