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/

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@397525 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/channels/sig_pri.c b/channels/sig_pri.c
index fcea9b57c2aaa4459f5e5d6e4be7d542d6ba4357..c220392f0e666cb4ce6aac9949ef067984876842 100644 (file)
--- a/channels/sig_pri.c
+++ b/channels/sig_pri.c
@@ -7989,7 +7989,7 @@ void sig_pri_cli_show_span(int fd, int *dchannels, struct sig_pri_span *pri)
                        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);

diff --git a/include/asterisk/astmm.h b/include/asterisk/astmm.h
index c2a717550e9bc4ac0af57d8ec189fd9185c29896..1b008120a71f4dd68de982cb9fed2908d54f5106 100644 (file)
--- a/include/asterisk/astmm.h
+++ b/include/asterisk/astmm.h
@@ -54,6 +54,12 @@ extern "C" {
 #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);
 void *__ast_malloc(size_t size, const char *file, int lineno, const char *func);

diff --git a/include/asterisk/lock.h b/include/asterisk/lock.h
index 61b84d1a9d0fe0866cc0f17014adcfd1d7761571..8029ab12d13f759908a1de3445f779fbc771b762 100644 (file)
--- a/include/asterisk/lock.h
+++ b/include/asterisk/lock.h
@@ -289,22 +289,6 @@ void ast_remove_lock_info(void *lock_addr);
 #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().
  *

diff --git a/include/asterisk/logger.h b/include/asterisk/logger.h
index 1a9f6a52521463d7874c253d96c56240e50ad5dd..d4122aa08a064201742028627d183a43100efda0 100644 (file)
--- a/include/asterisk/logger.h
+++ b/include/asterisk/logger.h
@@ -304,7 +304,7 @@ void *ast_bt_destroy(struct ast_bt *bt);
  * \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);

diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h
index b4c154e38253623b11c2ae002d45fd79ed5d3eaa..4fbfb92ceb23da8cdd6636a4c15d2f14bed03e3f 100644 (file)
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -436,24 +436,20 @@ char *ast_process_quotes_and_slashes(char *start, char find, char replace_with);
 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);

diff --git a/main/astmm.c b/main/astmm.c
index 9396d09873e8a06b4f2f8d85d41365d74c44af72..f3e1b362908e2de9a4c20bfd851b8f3b256aa89b 100644 (file)
--- a/main/astmm.c
+++ b/main/astmm.c
@@ -157,6 +157,31 @@ AST_MUTEX_DEFINE_STATIC_NOTRACKING(reglock);
                }                                    \
        } 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
  *

diff --git a/main/astobj2.c b/main/astobj2.c
index 24179c45b885d8f6d1effa2a3cfd7419494cd6e5..79e9d6526a9afa2fc66aa945612fc12979134f9d 100644 (file)
--- a/main/astobj2.c
+++ b/main/astobj2.c
@@ -98,7 +98,7 @@ void ao2_bt(void)
        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
 

diff --git a/main/lock.c b/main/lock.c
index b4b9172306ace7a94d39103661399b44fdc495d6..7dcb5333585f4af3a76b17cd3cd2592b2ea1a2d0 100644 (file)
--- a/main/lock.c
+++ b/main/lock.c
@@ -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_* */
@@ -45,6 +46,22 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #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)
 {

diff --git a/main/logger.c b/main/logger.c
index eacb180dfc0ca57a106d1a0b5f340f2dafee5916..e10f4763b9425a8b0de99e8f452cc16af83ab740 100644 (file)
--- a/main/logger.c
+++ b/main/logger.c
@@ -1334,7 +1334,7 @@ void *ast_bt_destroy(struct ast_bt *bt)
 
 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 */
@@ -1354,9 +1354,12 @@ char **ast_bt_get_symbols(void **addresses, size_t num_frames)
 
 #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)))) {
+       eachlen = ast_calloc(num_frames, sizeof(*eachlen));
+       strings = ast_std_calloc(num_frames, sizeof(*strings));
+       if (!eachlen || !strings) {
+               ast_free(eachlen);
+               ast_std_free(strings);
                return NULL;
        }
 
@@ -1371,6 +1374,7 @@ char **ast_bt_get_symbols(void **addresses, size_t num_frames)
 
                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_log(LOG_DEBUG, "Failed to find asterisk binary for debug symbols.\n");
@@ -1379,11 +1383,11 @@ char **ast_bt_get_symbols(void **addresses, size_t num_frames)
                }
 
                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;
@@ -1392,9 +1396,9 @@ char **ast_bt_get_symbols(void **addresses, size_t num_frames)
                        }
 
                        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;
                                }
 
@@ -1402,14 +1406,16 @@ char **ast_bt_get_symbols(void **addresses, size_t num_frames)
                                        continue;
                                }
 
-                                /* file can possibly be null even with a success result from bfd_find_nearest_line */
-                                file = file ? file : "";
+                               /* file can possibly be null even with a success result from bfd_find_nearest_line */
+                               file = file ? file : "";
 
                                /* Stack trace output */
                                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;
                                        }
@@ -1431,9 +1437,7 @@ char **ast_bt_get_symbols(void **addresses, size_t num_frames)
                }
                if (bfdobj) {
                        bfd_close(bfdobj);
-                       if (syms) {
-                               ast_free(syms);
-                       }
+                       ast_free(syms);
                }
 
                /* Default output, if we cannot find the information within BFD */
@@ -1453,52 +1457,32 @@ char **ast_bt_get_symbols(void **addresses, size_t num_frames)
 
                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;
 }
@@ -1523,7 +1507,7 @@ void ast_backtrace(void)
                        ast_log(LOG_DEBUG, "#%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");
        }

diff --git a/main/utils.c b/main/utils.c
index 1dcf34767c6ce50139c1aea79947369b816e5402..ec016234e1038f8b8f52cebb915ed03ca2fd0129 100644 (file)
--- a/main/utils.c
+++ b/main/utils.c
@@ -851,7 +851,7 @@ static void append_backtrace_information(struct ast_str **str, struct ast_bt *bt
                        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");
        }