res/res_pjsip: Fix crash due to misuse of session->media between threads.

This patch makes sure that thread running ast_taskprocessor_execute
cannot suddenly dispose the session->media object making the other
threads (running pbx_thread / bridge_channel_ind_thread) crash when they
try to access the pointer to invalid memory. We were experiencing a crash due
to a misuse of session->media container between threads running
(bridge_channel_ind_thread/pbx_thread) and the thread running
ast_taskprocessor_execute. Depending on the SIP flow (during a disconnection)
and the threads' code path, the session->media container was being destroyed
(and set to NULL) by the thread running ast_taskprocessor_execute while the
thread running t38_framehook_read was still referring to it.
Now res_pjsip_t38 is referring a session_media in a datastore.

ASTERISK-28156

Change-Id: Ia92e2389b8d804bf205473e92ec06217e87ce237
13.25
Paulo Vicentini 7 years ago
parent 2c1ac524b8
commit c01d2f66ee

@ -79,6 +79,17 @@ static const struct ast_datastore_info t38_datastore = {
.destroy = t38_state_destroy,
};
static void session_media_dec(void *obj)
{
ao2_cleanup(obj);
}
/*! \brief Datastore for T.38 session_media information */
static const struct ast_datastore_info session_media_datastore = {
.type = "t38_session_media",
.destroy = session_media_dec,
};
/*! \brief Structure for T.38 parameters task data */
struct t38_parameters_task_data {
/*! \brief Session itself */
@ -120,6 +131,36 @@ static struct t38_parameters_task_data *t38_parameters_task_data_alloc(struct as
return data;
}
/*! \brief Helper function which retrieves a T.38 session_media from datastore */
static struct ast_sip_session_media *session_media_from_datastore_addref(struct ast_sip_session *session)
{
struct ast_datastore *datastore = NULL;
struct ast_sip_session_media *session_media = NULL;
if ((datastore = ast_sip_session_get_datastore(session, "t38_session_media"))) {
session_media = datastore->data;
ao2_ref(session_media, +1);
ao2_ref(datastore, -1);
}
return session_media;
}
/*! \brief Helper function which allocates datastore with a session media */
static struct ast_datastore *create_datastore_session_media(struct ast_sip_session *session, struct ast_sip_session_media *session_media)
{
struct ast_datastore *datastore = NULL;
if (!(datastore = ast_sip_session_alloc_datastore(&session_media_datastore, "t38_session_media"))
|| ast_sip_session_add_datastore(session, datastore)) {
return NULL;
}
datastore->data = session_media;
ao2_bump(session_media);
return datastore;
}
/*! \brief Helper function for changing the T.38 state */
static void t38_change_state(struct ast_sip_session *session, struct ast_sip_session_media *session_media,
struct t38_state *state, enum ast_sip_session_t38state new_state)
@ -198,7 +239,7 @@ static int t38_automatic_reject(void *obj)
{
RAII_VAR(struct ast_sip_session *, session, obj, ao2_cleanup);
RAII_VAR(struct ast_datastore *, datastore, ast_sip_session_get_datastore(session, "t38"), ao2_cleanup);
RAII_VAR(struct ast_sip_session_media *, session_media, ao2_find(session->media, "image", OBJ_KEY), ao2_cleanup);
RAII_VAR(struct ast_sip_session_media *, session_media, session_media_from_datastore_addref(session), ao2_cleanup);
if (!datastore) {
return 0;
@ -255,6 +296,10 @@ static int t38_initialize_session(struct ast_sip_session *session, struct ast_si
return 0;
}
if (!create_datastore_session_media(session, session_media)) {
return -1;
}
if (!(session_media->udptl = ast_udptl_new_with_bindaddr(NULL, NULL, 0, &address))) {
return -1;
}
@ -292,7 +337,8 @@ static int t38_reinvite_response_cb(struct ast_sip_session *session, pjsip_rx_da
{
struct pjsip_status_line status = rdata->msg_info.msg->line.status;
struct t38_state *state;
struct ast_sip_session_media *session_media = NULL;
RAII_VAR(struct ast_sip_session_media *, session_media,
session_media_from_datastore_addref(session), ao2_cleanup);
if (status.code / 100 <= 1) {
/* Ignore any non-final responses (1xx) */
@ -300,7 +346,7 @@ static int t38_reinvite_response_cb(struct ast_sip_session *session, pjsip_rx_da
}
if (!session->channel || !(state = t38_state_get_or_alloc(session)) ||
!(session_media = ao2_find(session->media, "image", OBJ_KEY))) {
!session_media) {
ast_log(LOG_WARNING, "Received %d response to T.38 re-invite on '%s' but state unavailable\n",
status.code,
session->channel ? ast_channel_name(session->channel) : "unknown channel");
@ -311,7 +357,6 @@ static int t38_reinvite_response_cb(struct ast_sip_session *session, pjsip_rx_da
t38_change_state(session, session_media, state,
(status.code / 100 == 2) ? T38_ENABLED : T38_REJECTED);
ao2_cleanup(session_media);
return 0;
}
@ -446,10 +491,7 @@ static struct ast_frame *t38_framehook_write(struct ast_channel *chan,
} else if (f->frametype == AST_FRAME_MODEM) {
struct ast_sip_session_media *session_media;
/* 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);
session_media = session_media_from_datastore_addref(session);
if (session_media && session_media->udptl) {
ast_udptl_write(session_media->udptl, f);
}
@ -466,10 +508,7 @@ static struct ast_frame *t38_framehook_read(struct ast_channel *chan,
if (ast_channel_fdno(session->channel) == 5) {
struct ast_sip_session_media *session_media;
/* 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);
session_media = session_media_from_datastore_addref(session);
if (session_media && session_media->udptl) {
f = ast_udptl_read(session_media->udptl);
}

Loading…
Cancel
Save