diff options
author | Richard Mudgett <rmudgett@digium.com> | 2017-04-29 16:11:21 -0500 |
---|---|---|
committer | Richard Mudgett <rmudgett@digium.com> | 2017-04-29 18:15:32 -0500 |
commit | 52e4f02b1a100a414161ee70c64b9af5993e26bb (patch) | |
tree | 458b707b8e90ee891690aba3189f0f1fe32a94db /res | |
parent | 09cde039a3e3871d66d6b1e78051fb143674d814 (diff) |
res_pjsip_t38.c: Fix deadlock in T.38 framehook.
A deadlock can happen between a channel lock and a pjsip session media
container lock. One thread is processing a reINVITE's SDP and walking
through the session's media container when it waits for the channel lock
to put the determined format capabilities onto the channel. The other
thread is writing a frame to the channel and processing the T.38 frame
hook. The T.38 frame hook then waits for the pjsip session's media
container lock. The two threads are now deadlocked.
* Made the T.38 frame hook release the channel lock before searching the
session's media container. This fix has been done to several other
frame hooks to fix deadlocks.
ASTERISK-26974 #close
Change-Id: Ie984a76ce00bef6ec9aa239010e51e8dd74c8186
Diffstat (limited to 'res')
-rw-r--r-- | res/res_pjsip_t38.c | 30 |
1 files changed, 20 insertions, 10 deletions
diff --git a/res/res_pjsip_t38.c b/res/res_pjsip_t38.c index 4f082d714..bb1641a44 100644 --- a/res/res_pjsip_t38.c +++ b/res/res_pjsip_t38.c @@ -398,7 +398,8 @@ static int t38_interpret_parameters(void *obj) } /*! \brief Frame hook callback for writing */ -static struct ast_frame *t38_framehook_write(struct ast_sip_session *session, struct ast_frame *f) +static struct ast_frame *t38_framehook_write(struct ast_channel *chan, + struct ast_sip_session *session, struct ast_frame *f) { if (f->frametype == AST_FRAME_CONTROL && f->subclass.integer == AST_CONTROL_T38_PARAMETERS && session->endpoint->media.t38.enabled) { @@ -412,27 +413,36 @@ static struct ast_frame *t38_framehook_write(struct ast_sip_session *session, st ao2_ref(data, -1); } } else if (f->frametype == AST_FRAME_MODEM) { - RAII_VAR(struct ast_sip_session_media *, session_media, NULL, ao2_cleanup); + struct ast_sip_session_media *session_media; - if ((session_media = ao2_find(session->media, "image", OBJ_KEY)) && - session_media->udptl) { + /* Avoid deadlock between chan and the session->media container lock */ + ast_channel_unlock(chan); + session_media = ao2_find(session->media, "image", OBJ_SEARCH_KEY); + ast_channel_lock(chan); + if (session_media && session_media->udptl) { ast_udptl_write(session_media->udptl, f); } + ao2_cleanup(session_media); } return f; } /*! \brief Frame hook callback for reading */ -static struct ast_frame *t38_framehook_read(struct ast_sip_session *session, struct ast_frame *f) +static struct ast_frame *t38_framehook_read(struct ast_channel *chan, + struct ast_sip_session *session, struct ast_frame *f) { if (ast_channel_fdno(session->channel) == 5) { - RAII_VAR(struct ast_sip_session_media *, session_media, NULL, ao2_cleanup); + struct ast_sip_session_media *session_media; - if ((session_media = ao2_find(session->media, "image", OBJ_KEY)) && - session_media->udptl) { + /* Avoid deadlock between chan and the session->media container lock */ + ast_channel_unlock(chan); + session_media = ao2_find(session->media, "image", OBJ_SEARCH_KEY); + ast_channel_lock(chan); + if (session_media && session_media->udptl) { f = ast_udptl_read(session_media->udptl); } + ao2_cleanup(session_media); } return f; @@ -445,9 +455,9 @@ static struct ast_frame *t38_framehook(struct ast_channel *chan, struct ast_fram struct ast_sip_channel_pvt *channel = ast_channel_tech_pvt(chan); if (event == AST_FRAMEHOOK_EVENT_READ) { - f = t38_framehook_read(channel->session, f); + f = t38_framehook_read(chan, channel->session, f); } else if (event == AST_FRAMEHOOK_EVENT_WRITE) { - f = t38_framehook_write(channel->session, f); + f = t38_framehook_write(chan, channel->session, f); } return f; |