Skip to content
Snippets Groups Projects
Commit bc090677 authored by Richard Mudgett's avatar Richard Mudgett
Browse files

Fix potential reentrancy problems in chan_sip. 

Asterisk v1.8 and later was not as vulnerable to this issue.

* Made find_call() lock each private as it processes the found dialogs.
(Primary cause of ABE-2876)

* Made the other functions that traverse the dialogs container lock each
private as it examines them.

* Fix race condition in sip_call() if the thread that sent the INVITE is
held up long enough for a response to be processed.  The p->initid for the
INVITE retransmission could be added after it was canceled by the response
processing.

* Made __sip_destroy() clean up resource pointers after freeing.  This is
primarily defensive in case someone has a stale private pointer.

* Removed redundant memset() in reqprep().  The call to init_req() already
does the memset() and is the first reference to req in reqprep().

* Removed useless set of req.method in transmit_invite().  The calls to
initreqprep() and reqprep() have to do this because they memset() the req.

JIRA ABE-2876

..........

Merged -r373423 from https://origsvn.digium.com/svn/asterisk/be/branches/C.3-bier
........

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

Merged revisions 373466 from http://svn.asterisk.org/svn/asterisk/branches/10
........

Merged revisions 373469 from http://svn.asterisk.org/svn/asterisk/branches/11


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@373471 65c4cc65-6c06-0410-ace0-fbb531ad65f3
parent f6e04062
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 69 additions and 15 deletions
channels/chan_sip.c
+ 69
− 15
View file @ bc090677
...@@ -6074,9 +6074,10 @@ static int sip_call(struct ast_channel *ast, const char *dest, int timeout) ...@@ -6074,9 +6074,10 @@ static int sip_call(struct ast_channel *ast, const char *dest, int timeout)
} }
   
xmitres = transmit_invite(p, SIP_INVITE, 1, 2, uri); xmitres = transmit_invite(p, SIP_INVITE, 1, 2, uri);
sip_pvt_unlock(p); if (xmitres == XMIT_ERROR) {
if (xmitres == XMIT_ERROR) sip_pvt_unlock(p);
return -1; return -1;
}
p->invitestate = INV_CALLING; p->invitestate = INV_CALLING;
   
/* Initialize auto-congest time */ /* Initialize auto-congest time */
...@@ -6084,6 +6085,7 @@ static int sip_call(struct ast_channel *ast, const char *dest, int timeout) ...@@ -6084,6 +6085,7 @@ static int sip_call(struct ast_channel *ast, const char *dest, int timeout)
dialog_unref(_data, "dialog ptr dec when SCHED_REPLACE del op succeeded"), dialog_unref(_data, "dialog ptr dec when SCHED_REPLACE del op succeeded"),
dialog_unref(p, "dialog ptr dec when SCHED_REPLACE add failed"), dialog_unref(p, "dialog ptr dec when SCHED_REPLACE add failed"),
dialog_ref(p, "dialog ptr inc when SCHED_REPLACE add succeeded") ); dialog_ref(p, "dialog ptr inc when SCHED_REPLACE add succeeded") );
sip_pvt_unlock(p);
} }
return res; return res;
} }
...@@ -6187,6 +6189,7 @@ void __sip_destroy(struct sip_pvt *p, int lockowner, int lockdialoglist) ...@@ -6187,6 +6189,7 @@ void __sip_destroy(struct sip_pvt *p, int lockowner, int lockdialoglist)
if (p->mwi) { if (p->mwi) {
p->mwi->call = NULL; p->mwi->call = NULL;
p->mwi = NULL;
} }
   
if (dumphistory) if (dumphistory)
...@@ -6197,30 +6200,38 @@ void __sip_destroy(struct sip_pvt *p, int lockowner, int lockdialoglist) ...@@ -6197,30 +6200,38 @@ void __sip_destroy(struct sip_pvt *p, int lockowner, int lockdialoglist)
ao2_ref(p->options->outboundproxy, -1); ao2_ref(p->options->outboundproxy, -1);
} }
ast_free(p->options); ast_free(p->options);
p->options = NULL;
} }
   
if (p->notify) { if (p->notify) {
ast_variables_destroy(p->notify->headers); ast_variables_destroy(p->notify->headers);
ast_free(p->notify->content); ast_free(p->notify->content);
ast_free(p->notify); ast_free(p->notify);
p->notify = NULL;
} }
if (p->rtp) { if (p->rtp) {
ast_rtp_instance_destroy(p->rtp); ast_rtp_instance_destroy(p->rtp);
p->rtp = NULL;
} }
if (p->vrtp) { if (p->vrtp) {
ast_rtp_instance_destroy(p->vrtp); ast_rtp_instance_destroy(p->vrtp);
p->vrtp = NULL;
} }
if (p->trtp) { if (p->trtp) {
ast_rtp_instance_destroy(p->trtp); ast_rtp_instance_destroy(p->trtp);
p->trtp = NULL;
} }
if (p->udptl) if (p->udptl) {
ast_udptl_destroy(p->udptl); ast_udptl_destroy(p->udptl);
p->udptl = NULL;
}
if (p->refer) { if (p->refer) {
if (p->refer->refer_call) { if (p->refer->refer_call) {
p->refer->refer_call = dialog_unref(p->refer->refer_call, "unref dialog p->refer->refer_call"); p->refer->refer_call = dialog_unref(p->refer->refer_call, "unref dialog p->refer->refer_call");
} }
ast_string_field_free_memory(p->refer); ast_string_field_free_memory(p->refer);
ast_free(p->refer); ast_free(p->refer);
p->refer = NULL;
} }
if (p->route) { if (p->route) {
free_old_route(p->route); free_old_route(p->route);
...@@ -6274,6 +6285,7 @@ void __sip_destroy(struct sip_pvt *p, int lockowner, int lockdialoglist) ...@@ -6274,6 +6285,7 @@ void __sip_destroy(struct sip_pvt *p, int lockowner, int lockdialoglist)
ast_string_field_free_memory(p); ast_string_field_free_memory(p);
   
ast_cc_config_params_destroy(p->cc_params); ast_cc_config_params_destroy(p->cc_params);
p->cc_params = NULL;
   
if (p->epa_entry) { if (p->epa_entry) {
ao2_ref(p->epa_entry, -1); ao2_ref(p->epa_entry, -1);
...@@ -8622,10 +8634,34 @@ static enum match_req_res match_req_to_dialog(struct sip_pvt *sip_pvt_ptr, struc ...@@ -8622,10 +8634,34 @@ static enum match_req_res match_req_to_dialog(struct sip_pvt *sip_pvt_ptr, struc
static void forked_invite_init(struct sip_request *req, const char *new_theirtag, struct sip_pvt *original, struct ast_sockaddr *addr) static void forked_invite_init(struct sip_request *req, const char *new_theirtag, struct sip_pvt *original, struct ast_sockaddr *addr)
{ {
struct sip_pvt *p; struct sip_pvt *p;
const char *callid;
struct ast_callid *logger_callid;
   
if (!(p = sip_alloc(original->callid, addr, 1, SIP_INVITE, req, original->logger_callid))) { sip_pvt_lock(original);
callid = ast_strdupa(original->callid);
logger_callid = original->logger_callid;
if (logger_callid) {
ast_callid_ref(logger_callid);
}
sip_pvt_unlock(original);
p = sip_alloc(callid, addr, 1, SIP_INVITE, req, logger_callid);
if (logger_callid) {
ast_callid_unref(logger_callid);
}
if (!p) {
return; /* alloc error */ return; /* alloc error */
} }
/* Lock p and original private structures. */
sip_pvt_lock(p);
while (sip_pvt_trylock(original)) {
/* Can't use DEADLOCK_AVOIDANCE since p is an ao2 object */
sip_pvt_unlock(p);
sched_yield();
sip_pvt_lock(p);
}
p->invitestate = INV_TERMINATED; p->invitestate = INV_TERMINATED;
p->ocseq = original->ocseq; p->ocseq = original->ocseq;
p->branch = original->branch; p->branch = original->branch;
...@@ -8637,6 +8673,9 @@ static void forked_invite_init(struct sip_request *req, const char *new_theirtag ...@@ -8637,6 +8673,9 @@ static void forked_invite_init(struct sip_request *req, const char *new_theirtag
ast_string_field_set(p, uri, original->uri); ast_string_field_set(p, uri, original->uri);
ast_string_field_set(p, our_contact, original->our_contact); ast_string_field_set(p, our_contact, original->our_contact);
ast_string_field_set(p, fullcontact, original->fullcontact); ast_string_field_set(p, fullcontact, original->fullcontact);
sip_pvt_unlock(original);
parse_ok_contact(p, req); parse_ok_contact(p, req);
build_route(p, req, 1, 0); build_route(p, req, 1, 0);
   
...@@ -8644,6 +8683,7 @@ static void forked_invite_init(struct sip_request *req, const char *new_theirtag ...@@ -8644,6 +8683,7 @@ static void forked_invite_init(struct sip_request *req, const char *new_theirtag
transmit_request(p, SIP_BYE, 0, XMIT_RELIABLE, TRUE); transmit_request(p, SIP_BYE, 0, XMIT_RELIABLE, TRUE);
   
pvt_set_needdestroy(p, "forked request"); /* this dialog will terminate once the BYE is responed to or times out. */ pvt_set_needdestroy(p, "forked request"); /* this dialog will terminate once the BYE is responed to or times out. */
sip_pvt_unlock(p);
dialog_unref(p, "setup forked invite termination"); dialog_unref(p, "setup forked invite termination");
} }
   
...@@ -8823,7 +8863,9 @@ static struct sip_pvt *find_call(struct sip_request *req, struct ast_sockaddr *a ...@@ -8823,7 +8863,9 @@ static struct sip_pvt *find_call(struct sip_request *req, struct ast_sockaddr *a
   
/* Iterate a list of dialogs already matched by Call-id */ /* Iterate a list of dialogs already matched by Call-id */
while (iterator && (sip_pvt_ptr = ao2_iterator_next(iterator))) { while (iterator && (sip_pvt_ptr = ao2_iterator_next(iterator))) {
sip_pvt_lock(sip_pvt_ptr);
found = match_req_to_dialog(sip_pvt_ptr, &args); found = match_req_to_dialog(sip_pvt_ptr, &args);
sip_pvt_unlock(sip_pvt_ptr);
   
switch (found) { switch (found) {
case SIP_REQ_MATCH: case SIP_REQ_MATCH:
...@@ -8847,6 +8889,7 @@ static struct sip_pvt *find_call(struct sip_request *req, struct ast_sockaddr *a ...@@ -8847,6 +8889,7 @@ static struct sip_pvt *find_call(struct sip_request *req, struct ast_sockaddr *a
case SIP_REQ_NOT_MATCH: case SIP_REQ_NOT_MATCH:
default: default:
dialog_unref(sip_pvt_ptr, "pvt did not match incoming SIP msg, unref from search"); dialog_unref(sip_pvt_ptr, "pvt did not match incoming SIP msg, unref from search");
break;
} }
} }
if (iterator) { if (iterator) {
...@@ -11394,8 +11437,6 @@ static int reqprep(struct sip_request *req, struct sip_pvt *p, int sipmethod, ui ...@@ -11394,8 +11437,6 @@ static int reqprep(struct sip_request *req, struct sip_pvt *p, int sipmethod, ui
int is_strict = FALSE; /*!< Strict routing flag */ int is_strict = FALSE; /*!< Strict routing flag */
int is_outbound = ast_test_flag(&p->flags[0], SIP_OUTGOING); /* Session direction */ int is_outbound = ast_test_flag(&p->flags[0], SIP_OUTGOING); /* Session direction */
   
memset(req, 0, sizeof(struct sip_request));
snprintf(p->lastmsg, sizeof(p->lastmsg), "Tx: %s", sip_methods[sipmethod].text); snprintf(p->lastmsg, sizeof(p->lastmsg), "Tx: %s", sip_methods[sipmethod].text);
   
if (!seqno) { if (!seqno) {
...@@ -13052,19 +13093,21 @@ static void copy_request(struct sip_request *dst, const struct sip_request *src) ...@@ -13052,19 +13093,21 @@ static void copy_request(struct sip_request *dst, const struct sip_request *src)
   
/* copy the entire request then restore the original data and content /* copy the entire request then restore the original data and content
* members from the dst request */ * members from the dst request */
memcpy(dst, src, sizeof(*dst)); *dst = *src;
dst->data = duplicate; dst->data = duplicate;
dst->content = duplicate_content; dst->content = duplicate_content;
   
/* copy the data into the dst request */ /* copy the data into the dst request */
if (!dst->data && !(dst->data = ast_str_create(ast_str_strlen(src->data) + 1))) if (!dst->data && !(dst->data = ast_str_create(ast_str_strlen(src->data) + 1))) {
return; return;
}
ast_str_copy_string(&dst->data, src->data); ast_str_copy_string(&dst->data, src->data);
   
/* copy the content into the dst request (if it exists) */ /* copy the content into the dst request (if it exists) */
if (src->content) { if (src->content) {
if (!dst->content && !(dst->content = ast_str_create(ast_str_strlen(src->content) + 1))) if (!dst->content && !(dst->content = ast_str_create(ast_str_strlen(src->content) + 1))) {
return; return;
}
ast_str_copy_string(&dst->content, src->content); ast_str_copy_string(&dst->content, src->content);
} }
} }
...@@ -13565,8 +13608,7 @@ static int transmit_invite(struct sip_pvt *p, int sipmethod, int sdp, int init, ...@@ -13565,8 +13608,7 @@ static int transmit_invite(struct sip_pvt *p, int sipmethod, int sdp, int init,
{ {
struct sip_request req; struct sip_request req;
struct ast_variable *var; struct ast_variable *var;
req.method = sipmethod;
if (init) {/* Bump branch even on initial requests */ if (init) {/* Bump branch even on initial requests */
p->branch ^= ast_random(); p->branch ^= ast_random();
p->invite_branch = p->branch; p->invite_branch = p->branch;
...@@ -19833,13 +19875,18 @@ static int show_chanstats_cb(void *__cur, void *__arg, int flags) ...@@ -19833,13 +19875,18 @@ static int show_chanstats_cb(void *__cur, void *__arg, int flags)
char durbuf[10]; char durbuf[10];
int duration; int duration;
int durh, durm, durs; int durh, durm, durs;
struct ast_channel *c = cur->owner; struct ast_channel *c;
struct __show_chan_arg *arg = __arg; struct __show_chan_arg *arg = __arg;
int fd = arg->fd; int fd = arg->fd;
   
sip_pvt_lock(cur);
c = cur->owner;
   
if (cur->subscribed != NONE) /* Subscriptions */ if (cur->subscribed != NONE) {
/* Subscriptions */
sip_pvt_unlock(cur);
return 0; /* don't care, we scan all channels */ return 0; /* don't care, we scan all channels */
}
   
if (!cur->rtp) { if (!cur->rtp) {
if (sipdebug) { if (sipdebug) {
...@@ -19848,10 +19895,12 @@ static int show_chanstats_cb(void *__cur, void *__arg, int flags) ...@@ -19848,10 +19895,12 @@ static int show_chanstats_cb(void *__cur, void *__arg, int flags)
invitestate2string[cur->invitestate].desc, invitestate2string[cur->invitestate].desc,
"-- No RTP active"); "-- No RTP active");
} }
sip_pvt_unlock(cur);
return 0; /* don't care, we scan all channels */ return 0; /* don't care, we scan all channels */
} }
   
if (ast_rtp_instance_get_stats(cur->rtp, &stats, AST_RTP_INSTANCE_STAT_ALL)) { if (ast_rtp_instance_get_stats(cur->rtp, &stats, AST_RTP_INSTANCE_STAT_ALL)) {
sip_pvt_unlock(cur);
ast_log(LOG_WARNING, "Could not get RTP stats.\n"); ast_log(LOG_WARNING, "Could not get RTP stats.\n");
return 0; return 0;
} }
...@@ -19882,6 +19931,7 @@ static int show_chanstats_cb(void *__cur, void *__arg, int flags) ...@@ -19882,6 +19931,7 @@ static int show_chanstats_cb(void *__cur, void *__arg, int flags)
stats.txjitter stats.txjitter
); );
arg->numchans++; arg->numchans++;
sip_pvt_unlock(cur);
   
return 0; /* don't care, we scan all channels */ return 0; /* don't care, we scan all channels */
} }
...@@ -20226,8 +20276,11 @@ static int show_channels_cb(void *__cur, void *__arg, int flags) ...@@ -20226,8 +20276,11 @@ static int show_channels_cb(void *__cur, void *__arg, int flags)
{ {
struct sip_pvt *cur = __cur; struct sip_pvt *cur = __cur;
struct __show_chan_arg *arg = __arg; struct __show_chan_arg *arg = __arg;
const struct ast_sockaddr *dst = sip_real_dst(cur); const struct ast_sockaddr *dst;
sip_pvt_lock(cur);
dst = sip_real_dst(cur);
/* XXX indentation preserved to reduce diff. Will be fixed later */ /* XXX indentation preserved to reduce diff. Will be fixed later */
if (cur->subscribed == NONE && !arg->subscriptions) { if (cur->subscribed == NONE && !arg->subscriptions) {
/* set if SIP transfer in progress */ /* set if SIP transfer in progress */
...@@ -20262,6 +20315,7 @@ static int show_channels_cb(void *__cur, void *__arg, int flags) ...@@ -20262,6 +20315,7 @@ static int show_channels_cb(void *__cur, void *__arg, int flags)
); );
arg->numchans++; arg->numchans++;
} }
sip_pvt_unlock(cur);
return 0; /* don't care, we scan all channels */ return 0; /* don't care, we scan all channels */
} }
   
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment