Skip to content

Commit

Permalink
dialog: Fix some locking corner-cases
Browse files Browse the repository at this point in the history
    * Re-INVITE pinging: synchronize writers and readers for the
       dlg->legs->adv_contact string, as well as the entire dlg->legs
       struct.  Example subtle race conditions:

	 1. setting $DLG_timeout after t_relay() (read op) vs.
	    concurrently processing a 200 OK (write op)

	 2. calling MI "dlg_push_var" after t_relay() (read op) vs.
	    concurrently processing a 200 OK (write op)

	 3. in parallel forking, dlg_onreq_out() (read op) vs.
	    concurrently processing 200 OK for prev. branch (write op)

    * DB load: to be 100% accurate, do not use ref_unsafe(), as we
       don't hold the lock -- use the safe version instead!
  • Loading branch information
liviuchircu committed Feb 24, 2020
1 parent 4e2f1b4 commit 0dc9681
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 11 deletions.
6 changes: 3 additions & 3 deletions modules/dialog/dlg_db_handler.c
Expand Up @@ -709,7 +709,7 @@ static int load_dialog_info_from_db(int dlg_hash_size)
LM_CRIT("Unable to insert dlg %p into ping timer\n",dlg);
else {
/* reference dialog as kept in ping timer list */
ref_dlg_unsafe(dlg, 1);
ref_dlg(dlg, 1);
}
}

Expand All @@ -723,7 +723,7 @@ static int load_dialog_info_from_db(int dlg_hash_size)
"ping timer\n", dlg);
else {
/* reference dialog as kept in reinvite ping timer list */
ref_dlg_unsafe(dlg, 1);
ref_dlg(dlg, 1);
}
}

Expand All @@ -735,7 +735,7 @@ static int load_dialog_info_from_db(int dlg_hash_size)

if (dlg_db_mode == DB_MODE_DELAYED) {
/* to be later removed by timer */
ref_dlg_unsafe(dlg, 1);
ref_dlg(dlg, 1);
}

if (dlg->state==DLG_STATE_CONFIRMED_NA ||
Expand Down
35 changes: 28 additions & 7 deletions modules/dialog/dlg_handlers.c
Expand Up @@ -361,6 +361,8 @@ static inline void push_reply_in_dialog(struct sip_msg *rpl, struct cell* t,
if (tag.len==0 && rpl->REPLY_STATUS<200 )
return;

dlg_lock_dlg(dlg);

/* is the totag already known ?? */
for(leg=DLG_FIRST_CALLEE_LEG ; leg<dlg->legs_no[DLG_LEGS_USED] ; leg++ ) {
if ( dlg->legs[leg].tag.len==tag.len &&
Expand All @@ -379,7 +381,7 @@ static inline void push_reply_in_dialog(struct sip_msg *rpl, struct cell* t,
leg = dlg_clone_callee_leg(dlg, leg);
if (leg < 0) {
LM_ERR("failed to add callee leg!\n");
return;
goto out;
}
}

Expand All @@ -388,7 +390,7 @@ static inline void push_reply_in_dialog(struct sip_msg *rpl, struct cell* t,
if (update_leg_info(leg, dlg, rpl, &tag,extract_mangled_fromuri(mangled_from),
extract_mangled_touri(mangled_to)) !=0) {
LM_ERR("could not add further info to the dialog\n");
return;
goto out;
}

routing_info:
Expand Down Expand Up @@ -424,6 +426,9 @@ static inline void push_reply_in_dialog(struct sip_msg *rpl, struct cell* t,
if( rr_set.s )
pkg_free( rr_set.s);
}

out:
dlg_unlock_dlg(dlg);
}

static void _dlg_setup_reinvite_callbacks(struct cell *t, struct sip_msg *req,
Expand Down Expand Up @@ -707,8 +712,7 @@ static void dlg_update_out_sdp(struct dlg_cell *dlg, int in_leg, int out_leg, st
{
str sdp;
char *tmp;
str *in_sdp = &dlg->legs[in_leg].in_sdp;
str *out_sdp = &dlg->legs[out_leg].out_sdp;
str *in_sdp, *out_sdp;

if (get_body(msg,&sdp) < 0) {
LM_ERR("Failed to extract SDP \n");
Expand All @@ -717,6 +721,10 @@ static void dlg_update_out_sdp(struct dlg_cell *dlg, int in_leg, int out_leg, st
}

dlg_lock_dlg(dlg);

in_sdp = &dlg->legs[in_leg].in_sdp;
out_sdp = &dlg->legs[out_leg].out_sdp;

if (in_sdp->len == sdp.len &&
memcmp(in_sdp->s, sdp.s, sdp.len) == 0) {
/* we have the same sdp in outbound as the one in inbound */
Expand Down Expand Up @@ -1045,13 +1053,16 @@ static void dlg_onreply_out(struct cell* t, int type, struct tmcb_params *ps)
contact.s = msg->contact->name.s;
contact.len = msg->contact->len;

dlg_lock_dlg(dlg);
if (shm_str_sync(&dlg->legs[DLG_CALLER_LEG].adv_contact,
&contact) != 0) {
dlg_unlock_dlg(dlg);
LM_ERR("No more shm mem for outgoing contact hdr\n");
free_sip_msg(msg);
pkg_free(msg);
return;
}
dlg_unlock_dlg(dlg);
}
}

Expand Down Expand Up @@ -1173,6 +1184,7 @@ static void dlg_onreq_out(struct cell* t, int type, struct tmcb_params *ps)
struct dlg_cell *dlg;
struct dlg_leg *leg;
str buffer, contact;
int callee_leg;

buffer.s = ((str*)ps->extra1)->s;
buffer.len = ((str*)ps->extra1)->len;
Expand All @@ -1197,16 +1209,24 @@ static void dlg_onreq_out(struct cell* t, int type, struct tmcb_params *ps)
goto out_free;
}

/* we get called exactly once for each outgoing branch, so we can safely
* start creating each leg */
/*
* - we get called exactly once for each outgoing branch
* - in parallel forking, we may concurrently run with reply code!
*/
dlg_lock_dlg(dlg);

if (ensure_leg_array(dlg->legs_no[DLG_LEGS_USED] + 1, dlg) != 0)
goto out_free;

/* store the caller SDP into each callee leg, useful for Re-INVITE pings */
leg = &dlg->legs[dlg->legs_no[DLG_LEGS_USED]];
callee_leg = dlg->legs_no[DLG_LEGS_USED];

dlg_unlock_dlg(dlg);

dlg_update_out_sdp(dlg, DLG_CALLER_LEG, dlg->legs_no[DLG_LEGS_USED], msg);
dlg_update_out_sdp(dlg, DLG_CALLER_LEG, callee_leg, msg);

dlg_lock_dlg(dlg);

/* save the outgoing contact only if TH */
if (dlg->mod_flags & TOPOH_ONGOING) {
Expand All @@ -1228,6 +1248,7 @@ static void dlg_onreq_out(struct cell* t, int type, struct tmcb_params *ps)
dlg->legs_no[DLG_LEGS_USED]++;

out_free:
dlg_unlock_dlg(dlg);
free_sip_msg(msg);
pkg_free(msg);
}
Expand Down
1 change: 0 additions & 1 deletion modules/dialog/dlg_replication.c
Expand Up @@ -721,7 +721,6 @@ void replicate_dialog_updated(struct dlg_cell *dlg)
LM_ERR("Failed to replicate updated dialog\n");
end:
dlg_unlock_dlg(dlg);
return;
}

/**
Expand Down

0 comments on commit 0dc9681

Please sign in to comment.