diff options
author | Richard Mudgett <rmudgett@digium.com> | 2011-12-16 21:10:19 +0000 |
---|---|---|
committer | Richard Mudgett <rmudgett@digium.com> | 2011-12-16 21:10:19 +0000 |
commit | b05d4603c4506bae4a065409937fa7567df9bc9f (patch) | |
tree | b38e1b54c4e4d447a650211787e351a4d26835f5 | |
parent | 8baea2b35e96d1c51756c00aff322a4e1af858a0 (diff) |
Fix crash during CDR update.
The ast_cdr_setcid() and ast_cdr_update() were shown in ASTERISK-18836 to
be called by different threads for the same channel. The channel driver
thread and the PBX thread running dialplan.
* Add lock protection around CDR API calls that access an ast_channel
pointer.
(closes issue ASTERISK-18836)
Reported by: gpluser
Review: https://reviewboard.asterisk.org/r/1628/
........
Merged revisions 348362 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 348363 from http://svn.asterisk.org/svn/asterisk/branches/10
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@348364 65c4cc65-6c06-0410-ace0-fbb531ad65f3
-rw-r--r-- | apps/app_authenticate.c | 15 | ||||
-rw-r--r-- | apps/app_followme.c | 16 | ||||
-rw-r--r-- | apps/app_queue.c | 2 | ||||
-rw-r--r-- | funcs/func_cdr.c | 20 | ||||
-rw-r--r-- | include/asterisk/cdr.h | 32 | ||||
-rw-r--r-- | main/channel.c | 23 | ||||
-rw-r--r-- | main/features.c | 14 | ||||
-rw-r--r-- | main/pbx.c | 11 | ||||
-rw-r--r-- | res/res_monitor.c | 6 |
9 files changed, 109 insertions, 30 deletions
diff --git a/apps/app_authenticate.c b/apps/app_authenticate.c index 540e3df1c..3bdf48390 100644 --- a/apps/app_authenticate.c +++ b/apps/app_authenticate.c @@ -213,14 +213,20 @@ static int auth_exec(struct ast_channel *chan, const char *data) continue; ast_md5_hash(md5passwd, passwd); if (!strcmp(md5passwd, md5secret)) { - if (ast_test_flag(&flags,OPT_ACCOUNT)) + if (ast_test_flag(&flags,OPT_ACCOUNT)) { + ast_channel_lock(chan); ast_cdr_setaccount(chan, buf); + ast_channel_unlock(chan); + } break; } } else { if (!strcmp(passwd, buf)) { - if (ast_test_flag(&flags, OPT_ACCOUNT)) + if (ast_test_flag(&flags, OPT_ACCOUNT)) { + ast_channel_lock(chan); ast_cdr_setaccount(chan, buf); + ast_channel_unlock(chan); + } break; } } @@ -242,8 +248,11 @@ static int auth_exec(struct ast_channel *chan, const char *data) } if ((retries < 3) && !res) { - if (ast_test_flag(&flags,OPT_ACCOUNT) && !ast_test_flag(&flags,OPT_MULTIPLE)) + if (ast_test_flag(&flags,OPT_ACCOUNT) && !ast_test_flag(&flags,OPT_MULTIPLE)) { + ast_channel_lock(chan); ast_cdr_setaccount(chan, passwd); + ast_channel_unlock(chan); + } if (!(res = ast_streamfile(chan, "auth-thankyou", chan->language))) res = ast_waitstream(chan, ""); } else { diff --git a/apps/app_followme.c b/apps/app_followme.c index bab4d1ba3..9ee2e334e 100644 --- a/apps/app_followme.c +++ b/apps/app_followme.c @@ -492,10 +492,12 @@ static void clear_caller(struct findme_user *tmpuser) if (tmpuser && tmpuser->ochan && tmpuser->state >= 0) { outbound = tmpuser->ochan; + ast_channel_lock(outbound); if (!outbound->cdr) { outbound->cdr = ast_cdr_alloc(); - if (outbound->cdr) + if (outbound->cdr) { ast_cdr_init(outbound->cdr, outbound); + } } if (outbound->cdr) { char tmp[256]; @@ -506,11 +508,15 @@ static void clear_caller(struct findme_user *tmpuser) ast_cdr_start(outbound->cdr); ast_cdr_end(outbound->cdr); /* If the cause wasn't handled properly */ - if (ast_cdr_disposition(outbound->cdr, outbound->hangupcause)) + if (ast_cdr_disposition(outbound->cdr, outbound->hangupcause)) { ast_cdr_failed(outbound->cdr); - } else + } + } else { ast_log(LOG_WARNING, "Unable to create Call Detail Record\n"); - ast_hangup(tmpuser->ochan); + } + ast_channel_unlock(outbound); + ast_hangup(outbound); + tmpuser->ochan = NULL; } } @@ -890,6 +896,7 @@ static void findmeexec(struct fm_args *tpargs) AST_LIST_INSERT_TAIL(findme_user_list, tmpuser, entry); } else { ast_verb(3, "couldn't reach at this number.\n"); + ast_channel_lock(outbound); if (!outbound->cdr) { outbound->cdr = ast_cdr_alloc(); } @@ -909,6 +916,7 @@ static void findmeexec(struct fm_args *tpargs) } else { ast_log(LOG_ERROR, "Unable to create Call Detail Record\n"); } + ast_channel_unlock(outbound); ast_hangup(outbound); ast_free(tmpuser); } diff --git a/apps/app_queue.c b/apps/app_queue.c index fe8723c54..de88dcaba 100644 --- a/apps/app_queue.c +++ b/apps/app_queue.c @@ -5116,9 +5116,9 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce if ((strcasecmp(cdr->uniqueid, qe->chan->uniqueid)) && (strcasecmp(cdr->linkedid, qe->chan->uniqueid)) && (newcdr = ast_cdr_dup(cdr))) { + ast_channel_lock(qe->chan); ast_cdr_init(newcdr, qe->chan); ast_cdr_reset(newcdr, 0); - ast_channel_lock(qe->chan); cdr = ast_cdr_append(cdr, newcdr); cdr = cdr->next; ast_channel_unlock(qe->chan); diff --git a/funcs/func_cdr.c b/funcs/func_cdr.c index 34fdd6736..dc7109659 100644 --- a/funcs/func_cdr.c +++ b/funcs/func_cdr.c @@ -197,17 +197,21 @@ static int cdr_read(struct ast_channel *chan, const char *cmd, char *parse, { char *ret; struct ast_flags flags = { 0 }; - struct ast_cdr *cdr = chan ? chan->cdr : NULL; + struct ast_cdr *cdr; AST_DECLARE_APP_ARGS(args, AST_APP_ARG(variable); AST_APP_ARG(options); ); - if (ast_strlen_zero(parse)) + if (ast_strlen_zero(parse) || !chan) return -1; - if (!cdr) + ast_channel_lock(chan); + cdr = chan->cdr; + if (!cdr) { + ast_channel_unlock(chan); return -1; + } AST_STANDARD_APP_ARGS(args, parse); @@ -255,13 +259,14 @@ static int cdr_read(struct ast_channel *chan, const char *cmd, char *parse, ast_test_flag(&flags, OPT_UNPARSED)); } + ast_channel_unlock(chan); return ret ? 0 : -1; } static int cdr_write(struct ast_channel *chan, const char *cmd, char *parse, const char *value) { - struct ast_cdr *cdr = chan ? chan->cdr : NULL; + struct ast_cdr *cdr; struct ast_flags flags = { 0 }; AST_DECLARE_APP_ARGS(args, AST_APP_ARG(variable); @@ -271,8 +276,12 @@ static int cdr_write(struct ast_channel *chan, const char *cmd, char *parse, if (ast_strlen_zero(parse) || !value || !chan) return -1; - if (!cdr) + ast_channel_lock(chan); + cdr = chan->cdr; + if (!cdr) { + ast_channel_unlock(chan); return -1; + } AST_STANDARD_APP_ARGS(args, parse); @@ -296,6 +305,7 @@ static int cdr_write(struct ast_channel *chan, const char *cmd, char *parse, /* No need to worry about the u flag, as all fields for which setting * 'u' would do anything are marked as readonly. */ + ast_channel_unlock(chan); return 0; } diff --git a/include/asterisk/cdr.h b/include/asterisk/cdr.h index e755fe30f..c2268e312 100644 --- a/include/asterisk/cdr.h +++ b/include/asterisk/cdr.h @@ -213,6 +213,7 @@ void ast_cdr_discard(struct ast_cdr *cdr); * \param cdr Call Detail Record to use for channel * \param chan Channel to bind CDR with * Initializes a CDR and associates it with a particular channel + * \note The channel should be locked before calling. * \return 0 by default */ int ast_cdr_init(struct ast_cdr *cdr, struct ast_channel *chan); @@ -222,6 +223,7 @@ int ast_cdr_init(struct ast_cdr *cdr, struct ast_channel *chan); * \param cdr Call Detail Record to use for channel * \param chan Channel to bind CDR with * Initializes a CDR and associates it with a particular channel + * \note The channel should be locked before calling. * \return 0 by default */ int ast_cdr_setcid(struct ast_cdr *cdr, struct ast_channel *chan); @@ -414,22 +416,40 @@ char *ast_cdr_flags2str(int flags); */ void ast_cdr_merge(struct ast_cdr *to, struct ast_cdr *from); -/*! \brief Set account code, will generate AMI event */ +/*! + * \brief Set account code, will generate AMI event + * \note The channel should be locked before calling. + */ int ast_cdr_setaccount(struct ast_channel *chan, const char *account); -/*! \brief Set the peer account */ +/*! + * \brief Set the peer account + * \note The channel should be locked before calling. + */ int ast_cdr_setpeeraccount(struct ast_channel *chan, const char *account); -/*! \brief Set AMA flags for channel */ +/*! + * \brief Set AMA flags for channel + * \note The channel should be locked before calling. + */ int ast_cdr_setamaflags(struct ast_channel *chan, const char *amaflags); -/*! \brief Set CDR user field for channel (stored in CDR) */ +/*! + * \brief Set CDR user field for channel (stored in CDR) + * \note The channel should be locked before calling. + */ int ast_cdr_setuserfield(struct ast_channel *chan, const char *userfield); -/*! \brief Append to CDR user field for channel (stored in CDR) */ +/*! + * \brief Append to CDR user field for channel (stored in CDR) + * \note The channel should be locked before calling. + */ int ast_cdr_appenduserfield(struct ast_channel *chan, const char *userfield); -/*! Update CDR on a channel */ +/*! + * \brief Update CDR on a channel + * \note The channel should be locked before calling. + */ int ast_cdr_update(struct ast_channel *chan); diff --git a/main/channel.c b/main/channel.c index bb1bd4e7c..57788b5bd 100644 --- a/main/channel.c +++ b/main/channel.c @@ -5435,7 +5435,9 @@ struct ast_channel *ast_call_forward(struct ast_channel *caller, struct ast_chan ast_channel_datastore_inherit(oh->parent_channel, new); } if (oh->account) { + ast_channel_lock(new); ast_cdr_setaccount(new, oh->account); + ast_channel_lock(new); } } else if (caller) { /* no outgoing helper so use caller if avaliable */ ast_channel_update_redirecting(caller, apr, NULL); @@ -5501,8 +5503,11 @@ struct ast_channel *__ast_request_and_dial(const char *type, struct ast_format_c ast_channel_inherit_variables(oh->parent_channel, chan); ast_channel_datastore_inherit(oh->parent_channel, chan); } - if (oh->account) - ast_cdr_setaccount(chan, oh->account); + if (oh->account) { + ast_channel_lock(chan); + ast_cdr_setaccount(chan, oh->account); + ast_channel_unlock(chan); + } } ast_set_callerid(chan, cid_num, cid_name, cid_num); @@ -5614,21 +5619,27 @@ struct ast_channel *__ast_request_and_dial(const char *type, struct ast_format_c *outstate = AST_CONTROL_ANSWER; if (res <= 0) { - if ( AST_CONTROL_RINGING == last_subclass ) + ast_channel_lock(chan); + if (AST_CONTROL_RINGING == last_subclass) { chan->hangupcause = AST_CAUSE_NO_ANSWER; - if (!chan->cdr && (chan->cdr = ast_cdr_alloc())) + } + if (!chan->cdr && (chan->cdr = ast_cdr_alloc())) { ast_cdr_init(chan->cdr, chan); + } if (chan->cdr) { char tmp[256]; + snprintf(tmp, sizeof(tmp), "%s/%s", type, (char *)data); - ast_cdr_setapp(chan->cdr,"Dial",tmp); + ast_cdr_setapp(chan->cdr, "Dial", tmp); ast_cdr_update(chan); ast_cdr_start(chan->cdr); ast_cdr_end(chan->cdr); /* If the cause wasn't handled properly */ - if (ast_cdr_disposition(chan->cdr,chan->hangupcause)) + if (ast_cdr_disposition(chan->cdr, chan->hangupcause)) { ast_cdr_failed(chan->cdr); + } } + ast_channel_unlock(chan); ast_hangup(chan); chan = NULL; } diff --git a/main/features.c b/main/features.c index 6f23bd357..bb925f73d 100644 --- a/main/features.c +++ b/main/features.c @@ -2303,6 +2303,7 @@ static int builtin_blindtransfer(struct ast_channel *chan, struct ast_channel *p pbx_builtin_setvar_helper(transferer, "BLINDTRANSFER", transferee->name); pbx_builtin_setvar_helper(transferee, "BLINDTRANSFER", transferer->name); finishup(transferee); + ast_channel_lock(transferer); if (!transferer->cdr) { /* this code should never get called (in a perfect world) */ transferer->cdr = ast_cdr_alloc(); if (transferer->cdr) { @@ -2310,6 +2311,7 @@ static int builtin_blindtransfer(struct ast_channel *chan, struct ast_channel *p ast_cdr_start(transferer->cdr); } } + ast_channel_unlock(transferer); if (transferer->cdr) { struct ast_cdr *swap = transferer->cdr; @@ -3911,11 +3913,15 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a /* copy the userfield from the B-leg to A-leg if applicable */ if (chan->cdr && peer->cdr && !ast_strlen_zero(peer->cdr->userfield)) { char tmp[256]; + + ast_channel_lock(chan); if (!ast_strlen_zero(chan->cdr->userfield)) { snprintf(tmp, sizeof(tmp), "%s;%s", chan->cdr->userfield, peer->cdr->userfield); ast_cdr_appenduserfield(chan, tmp); - } else + } else { ast_cdr_setuserfield(chan, peer->cdr->userfield); + } + ast_channel_unlock(chan); /* Don't delete the CDR; just disable it. */ ast_set_flag(peer->cdr, AST_CDR_FLAG_POST_DISABLED); we_disabled_peer_cdr = 1; @@ -3924,7 +3930,7 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a ast_copy_string(orig_peername,peer->name,sizeof(orig_peername)); if (!chan_cdr || (chan_cdr && !ast_test_flag(chan_cdr, AST_CDR_FLAG_POST_DISABLED))) { - + ast_channel_lock_both(chan, peer); if (chan_cdr) { ast_set_flag(chan_cdr, AST_CDR_FLAG_MAIN); ast_cdr_update(chan); @@ -3939,7 +3945,6 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a ast_copy_string(bridge_cdr->userfield, peer_cdr->userfield, sizeof(bridge_cdr->userfield)); } ast_cdr_setaccount(peer, chan->accountcode); - } else { /* better yet, in a xfer situation, find out why the chan cdr got zapped (pun unintentional) */ bridge_cdr = ast_cdr_alloc(); /* this should be really, really rare/impossible? */ @@ -3962,6 +3967,9 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a ast_cdr_start(bridge_cdr); } } + ast_channel_unlock(chan); + ast_channel_unlock(peer); + ast_debug(4, "bridge answer set, chan answer set\n"); /* peer_cdr->answer will be set when a macro runs on the peer; in that case, the bridge answer will be delayed while the diff --git a/main/pbx.c b/main/pbx.c index fc4b3d93a..3ae294fcc 100644 --- a/main/pbx.c +++ b/main/pbx.c @@ -5112,10 +5112,12 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c, set_ext_pri(c, "s", 1); } + ast_channel_lock(c); if (c->cdr) { /* allow CDR variables that have been collected after channel was created to be visible during call */ ast_cdr_update(c); } + ast_channel_unlock(c); for (;;) { char dst_exten[256]; /* buffer to accumulate digits */ int pos = 0; /* XXX should check bounds */ @@ -5222,8 +5224,11 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c, } /* Call timed out with no special extension to jump to. */ } - if (c->cdr) + ast_channel_lock(c); + if (c->cdr) { ast_cdr_update(c); + } + ast_channel_unlock(c); error = 1; break; } @@ -5327,10 +5332,12 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c, } } } + ast_channel_lock(c); if (c->cdr) { ast_verb(2, "CDR updated on %s\n",c->name); ast_cdr_update(c); } + ast_channel_unlock(c); } } @@ -9571,7 +9578,9 @@ static int pbx_builtin_resetcdr(struct ast_channel *chan, const char *data) static int pbx_builtin_setamaflags(struct ast_channel *chan, const char *data) { /* Copy the AMA Flags as specified */ + ast_channel_lock(chan); ast_cdr_setamaflags(chan, data ? data : ""); + ast_channel_unlock(chan); return 0; } diff --git a/res/res_monitor.c b/res/res_monitor.c index 61dc4a2d0..9dac05177 100644 --- a/res/res_monitor.c +++ b/res/res_monitor.c @@ -684,9 +684,13 @@ static int start_monitor_exec(struct ast_channel *chan, const char *data) if (urlprefix) { snprintf(tmp, sizeof(tmp), "%s/%s.%s", urlprefix, args.fname_base, ((strcmp(args.format, "gsm")) ? "wav" : "gsm")); - if (!chan->cdr && !(chan->cdr = ast_cdr_alloc())) + ast_channel_lock(chan); + if (!chan->cdr && !(chan->cdr = ast_cdr_alloc())) { + ast_channel_unlock(chan); return -1; + } ast_cdr_setuserfield(chan, tmp); + ast_channel_unlock(chan); } if (waitforbridge) { /* We must remove the "b" option if listed. In principle none of |