From: Matthew Jordan Date: Fri, 2 Nov 2012 15:23:12 +0000 (+0000) Subject: Multiple revisions 370205,370273,370360 X-Git-Tag: certified/1.8.15-cert1-rc1~3^2~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aed9f84e3f62f42362f0bbbe76d54c33c587fbb7;p=thirdparty%2Fasterisk.git Multiple revisions 370205,370273,370360 ........ r370205 | kpfleming | 2012-07-18 14:12:03 -0500 (Wed, 18 Jul 2012) | 18 lines Resolve severe memory leak in CEL logging modules. A customer reported a significant memory leak using Asterisk 1.8. They have tracked it down to ast_cel_fabricate_channel_from_event() in main/cel.c, which is called by both in-tree CEL logging modules (cel_custom.c and cel_sqlite3_custom.c) for each and every CEL event that they log. The cause was an incorrect assumption about how data attached to an ast_channel would be handled when the channel is destroyed; the data is now stored in a datastore attached to the channel, which is destroyed along with the channel at the proper time. (closes issue AST-916) Reported by: Thomas Arimont Review: https://reviewboard.asterisk.org/r/2053/ ........ r370273 | mjordan | 2012-07-19 17:00:14 -0500 (Thu, 19 Jul 2012) | 14 lines Fix compilation error when MALLOC_DEBUG is enabled To fix a memory leak in CEL, a channel datastore was introduced whose destruction function pointer was pointed to the ast_free macro. Without MALLOC_DEBUG enabled this compiles as fine, as ast_free is defined as free. With MALLOC_DEBUG enabled, however, ast_free takes on a definition from a different place then utils.h, and became undefined. This patch resolves this by using a reference to ast_free_ptr. When MALLOC_DEBUG is enabled, this calls ast_free; when MALLOC_DEBUG is not enabled, this is defined to be ast_free, which is defined to be free. (issue AST-916) Reported by: Thomas Arimont ........ r370360 | kpfleming | 2012-07-23 09:41:03 -0500 (Mon, 23 Jul 2012) | 10 lines Free any datastores attached to dummy channels. Revision 370205 added the use of a datastore attached to a dummy channel to resolve a memory leak, but ast_dummy_channel_destructor() in this branch did not free datastores, resulting in a continued (but slightly smaller) memory leak. This patch backports the change to free said datastores from the Asterisk trunk. (related to issue AST-916) ........ Merged revisions 370205,370273,370360 from http://svn.asterisk.org/svn/asterisk/branches/1.8 git-svn-id: https://origsvn.digium.com/svn/asterisk/certified/branches/1.8.15@375587 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- diff --git a/main/cel.c b/main/cel.c index fa69b02513..e9d05901a5 100644 --- a/main/cel.c +++ b/main/cel.c @@ -390,6 +390,14 @@ void ast_cel_check_retire_linkedid(struct ast_channel *chan) ao2_ref(lid, -1); } +/* Note that no 'chan_fixup' function is provided for this datastore type, + * because the channels that will use it will never be involved in masquerades. + */ +static const struct ast_datastore_info fabricated_channel_datastore = { + .type = "CEL fabricated channel", + .destroy = ast_free_ptr, +}; + struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event *event) { struct varshead *headp; @@ -399,6 +407,8 @@ struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event struct ast_cel_event_record record = { .version = AST_CEL_EVENT_RECORD_VERSION, }; + struct ast_datastore *datastore; + char *app_data; /* do not call ast_channel_alloc because this is not really a real channel */ if (!(tchan = ast_dummy_channel_alloc())) { @@ -461,10 +471,42 @@ struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event AST_LIST_INSERT_HEAD(headp, newvariable, entries); } - tchan->appl = ast_strdup(record.application_name); - tchan->data = ast_strdup(record.application_data); tchan->amaflags = record.amaflag; + /* We need to store an 'application name' and 'application + * data' on the channel for logging purposes, but the channel + * structure only provides a place to store pointers, and it + * expects these pointers to be pointing to data that does not + * need to be freed. This means that the channel's destructor + * does not attempt to free any storage that these pointers + * point to. However, we can't provide data in that form directly for + * these structure members. In order to ensure that these data + * elements have a lifetime that matches the channel's + * lifetime, we'll put them in a datastore attached to the + * channel, and set's the channel's pointers to point into the + * datastore. The datastore will then be automatically destroyed + * when the channel is destroyed. + */ + + if (!(datastore = ast_datastore_alloc(&fabricated_channel_datastore, NULL))) { + ast_channel_unref(tchan); + return NULL; + } + + if (!(app_data = ast_malloc(strlen(record.application_name) + strlen(record.application_data) + 2))) { + ast_datastore_free(datastore); + ast_channel_unref(tchan); + return NULL; + } + + tchan->appl = app_data; + tchan->data = app_data + strlen(record.application_name) + 1; + + strcpy((char *) tchan->appl, record.application_name); + strcpy((char *) tchan->data, record.application_data); + datastore->data = app_data; + ast_channel_datastore_add(tchan, datastore); + return tchan; } diff --git a/main/channel.c b/main/channel.c index c3d2bf8ae5..7be6560703 100644 --- a/main/channel.c +++ b/main/channel.c @@ -2499,6 +2499,13 @@ static void ast_dummy_channel_destructor(void *obj) struct ast_channel *chan = obj; struct ast_var_t *vardata; struct varshead *headp; + struct ast_datastore *datastore; + + /* Get rid of each of the data stores on the channel */ + while ((datastore = AST_LIST_REMOVE_HEAD(&chan->datastores, entry))) { + /* Free the data store */ + ast_datastore_free(datastore); + } headp = &chan->varshead;