Skip to content

Commit

Permalink
mid_registrar_save(): Fix shm memleak with no t_relay()
Browse files Browse the repository at this point in the history
Since mid_registrar_save() is stateful by definition, it needs the SIP
transaction to properly function, so always forcing its creation if the
REGISTER is about to be forwarded is not that significant of a change,
if any at all.

Thus, this patch fixes a SHM memleak on the following type of logic:

    mid_registrar_save();

    # script terminates without having created the transaction,
    # so mid_registrar's "prepped" data does not get freed on T deletion
    exit;
  • Loading branch information
liviuchircu committed Nov 10, 2020
1 parent e85b960 commit b14a7ea
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 6 deletions.
27 changes: 22 additions & 5 deletions modules/mid_registrar/save.c
Expand Up @@ -1861,6 +1861,28 @@ static int prepare_forward(struct sip_msg *msg, udomain_t *d,
{
struct mid_reg_info *mri;
struct to_body *to, *from;
int rc;

rc = tmb.t_newtran(msg);
switch (rc) {
case 1:
break;

case E_SCRIPT:
LM_DBG("%.*s transaction already exists, continuing...\n",
msg->REQ_METHOD_S.len, msg->REQ_METHOD_S.s);
break;

case 0:
LM_INFO("absorbing %.*s retransmission, use t_check_trans() "
"earlier\n", msg->REQ_METHOD_S.len, msg->REQ_METHOD_S.s);
return 0;

default:
LM_ERR("internal error %d while creating %.*s transaction\n",
rc, msg->REQ_METHOD_S.len, msg->REQ_METHOD_S.s);
return -1;
}

LM_DBG("from: '%.*s'\n", msg->from->body.len, msg->from->body.s);
LM_DBG("Call-ID: '%.*s'\n", msg->callid->body.len, msg->callid->body.s);
Expand Down Expand Up @@ -2471,11 +2493,6 @@ int mid_reg_save(struct sip_msg *msg, udomain_t *d, str *flags_str,
return -1;
}

if (((int (*)(struct sip_msg *))tmb.t_check_trans)(msg) == 0) {
LM_INFO("absorbing retransmission, use t_check_trans() earlier!\n");
return 0;
}

rerrno = R_FINE;
memset(&sctx, 0, sizeof sctx);

Expand Down
3 changes: 3 additions & 0 deletions modules/tm/t_hooks.h
Expand Up @@ -153,6 +153,9 @@ struct cell;
* to any other piece of code except the transaction cleanup routine. All
* registered callbacks of this type will be invoked by the cleanup routine,
* after which the transaction object will be freed.
* Note: if you schedule data for deletion using this callback, then you may
* have to ensure a transaction always exists beforehand, using
* tmb.t_newtran()
*
* TMCB_MSG_MATCHED_IN -- triggered whenever there is an
* incoming SIP message matching the transaction. It may be
Expand Down
2 changes: 1 addition & 1 deletion modules/tm/tm_load.h
Expand Up @@ -54,7 +54,7 @@ struct tm_binds {
* 1 (success)
* 0 (retransmission)
* < 0 (error)
* * E_SCRIPT (T already exists)
* * E_SCRIPT (the current transaction (@T) is already populated)
* * (others)
*/
tnewtran_f t_newtran;
Expand Down

0 comments on commit b14a7ea

Please sign in to comment.