diff options
author | Kevin P. Fleming <kpfleming@digium.com> | 2012-07-18 19:18:40 +0000 |
---|---|---|
committer | Kevin P. Fleming <kpfleming@digium.com> | 2012-07-18 19:18:40 +0000 |
commit | 4a4189b085d20baa88daf2fa5c7e191514791fc2 (patch) | |
tree | e5b180de28079100a4c1db74a2c1d571f1a858dd | |
parent | 79087cbbd515a108188b93cf732e5baa1b4f28d0 (diff) |
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/
........
Merged revisions 370205 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 370206 from http://svn.asterisk.org/svn/asterisk/branches/10
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@370211 65c4cc65-6c06-0410-ace0-fbb531ad65f3
-rw-r--r-- | main/cel.c | 46 |
1 files changed, 44 insertions, 2 deletions
diff --git a/main/cel.c b/main/cel.c index 95936bca6..8a708cc3b 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, +}; + struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event *event) { struct varshead *headp; @@ -400,6 +408,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())) { @@ -467,10 +477,42 @@ struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event AST_LIST_INSERT_HEAD(headp, newvariable, entries); } - ast_channel_appl_set(tchan, ast_strdup(record.application_name)); - ast_channel_data_set(tchan, ast_strdup(record.application_data)); ast_channel_amaflags_set(tchan, 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; + } + + ast_channel_appl_set(tchan, app_data); + ast_channel_data_set(tchan, app_data + strlen(record.application_name) + 1); + + strcpy((char *) ast_channel_appl(tchan), record.application_name); + strcpy((char *) ast_channel_data(tchan), record.application_data); + datastore->data = app_data; + ast_channel_datastore_add(tchan, datastore); + return tchan; } |