[CRASH] tm: SIGSEGV in add_lump_rpl() under load — unsynchronized reply_lump on the shared transaction request

[davegreeko2023]

Summary

tm's _reply_with_body() (modules/tm/t_reply.c) adds reply lumps to
trans->uas.request->reply_lump without holding the transaction reply lock.
trans->uas.request is a clone in shared memory, so when two processes build a
reply for the same transaction concurrently they race on that single linked list
and corrupt it, crashing with a SIGSEGV in add_lump_rpl() while walking
msg->reply_lump.

The lock_replies argument to _reply_with_body() is only honored later, inside
_reply_light(). The whole add_lump_rpl -> build_res_buf_from_sip_req -> unlink/free_lump_rpl window on the shared list runs unprotected.

Environment

• Version: opensips 4.1.0-dev, git 2cd2a45a0 (master), x86_64/linux
• Severity: crash (worker SIGSEGV; the attendant then tears down the process
  group, taking the proxy down). Recurs continuously under load.
• Reproducibility: high, under concurrency with replies that carry a body or
  extra headers (so a reply_lump is actually added). Trivially hit by any
  b2b_entities-based B2BUA, which forwards SDP-bearing replies via
  t_reply_with_body.

Backtrace

#0 add_lump_rpl (msg=0x..., s="Content-Type: application/sdp\r\nContact: sip:...", len=72, flags=2) at data_lump_rpl.c:80
#1 _reply_with_body (trans=0x..., code=200, ..., lock_replies=1) at t_reply.c:1801
#2 t_reply_with_body (...) at t_reply.c:1931
#3 _b2b_send_reply (...) at modules/b2b_entities/dlg.c:2151
#4 b2b_send_reply (...) at modules/b2b_entities/dlg.c:2197
... (B2BUA forwards callee 200 OK to caller via b2b_api.send_reply)
#13 reply_received (...) at t_reply.c:1767
#15 receive_msg (... "SIP/2.0 200 OK ...")

data_lump_rpl.c:80 is the list walk:

for (foo = msg->reply_lump; foo->next; foo = foo->next);

Root cause

static int _reply_with_body(struct cell *trans, ..., int lock_replies)
{
    struct sip_msg* p_msg = trans->uas.request;          /* SHARED shm clone */
    ...
    hdr_lump  = add_lump_rpl(p_msg, new_header->s, ..., LUMP_RPL_HDR);   /* mutates reply_lump */
    body_lump = add_lump_rpl(p_msg, body->s, ..., LUMP_RPL_BODY);       /* mutates reply_lump */
    ...
    rpl.s = build_res_buf_from_sip_req(code, text, ..., p_msg, ...);    /* reads  reply_lump */
    ...
    unlink_lump_rpl(p_msg, hdr_lump);  free_lump_rpl(hdr_lump);         /* mutates reply_lump */
    unlink_lump_rpl(p_msg, body_lump); free_lump_rpl(body_lump);
    ...
    ret = _reply_light(trans, rpl.s, rpl.len, ..., lock_replies, &bm);  /* lock taken onl
}

The existing comment already flags the shm/private-memory hazard ("since the msg
is a clone into shm memory ... I will remove the lumps by myself here"). It
correctly handles the single-process lifetime, but nothing serializes two
processes doing this for the same transaction.

Core-dump evidence of the race: the faulting process was inside the else
branch of add_lump_rpl (taken only when msg->reply_lump != NULL), walking the
list, yet msg->reply_lump read 0x0 in the dump. That is only possible if a
second process emptied the list (unlink_lump_rpl/free_lump_rpl) while the
first was mid-walk; the walk then dereferenced a freed foo->next.

Why a B2BUA triggers it readily: a b2b_entities B2BUA forwards the callee's
provisional/final replies to the caller's UAS transaction via t_reply_with_body
(needed to carry the SDP body plus Content-Type/Contact). Under load a caller
CANCEL or INVITE retransmission is handled on that same UAS transaction by a
different worker, and its reply build (e.g. 487) races the in-flight 200 OK
forward. Both call add_lump_rpl(trans->uas.request, ...) with no shared lock.

Proposed fix

Hold the transaction reply lock across the entire lump lifecycle (add -> build ->
free) and call _reply_light() without re-locking, gated on lock_replies
exactly as before:

--- a/modules/tm/t_reply.c
+++ b/modules/tm/t_reply.c
@@ static int _reply_with_body( struct cell *trans, ... )
      struct sip_msg* p_msg = trans->uas.request;
      str to_tag_rpl= {0, 0};

+     /* p_msg (trans->uas.request) is a SHARED-memory clone; two processes
+      * building a reply for the same transaction race on p_msg->reply_lump and
+      * corrupt it (SIGSEGV in add_lump_rpl). Serialize add -> build -> free with
+      * the reply lock; _reply_light below is called without re-locking. */
+     if (lock_replies)
+             LOCK_REPLIES( trans );
+
      /* add the lumps for new_header and for body (by bogdan) */
      ...
      ret=_reply_light( trans, rpl.s, rpl.len, code, to_tag_rpl.s, to_tag_rpl.len,
-                     lock_replies, &bm );
+                     0 /* lock already held above */, &bm );

      if (code>=200) set_kr(REQ_RPLD);

+     if (lock_replies)
+             UNLOCK_REPLIES( trans );
      return ret;
 error_1:
      if ( hdr_lump ) { unlink_lump_rpl( p_msg, hdr_lump); free_lump_rpl( hdr_lump ); }
 error:
+     if (lock_replies)
+             UNLOCK_REPLIES( trans );
      return -1;
 }

Notes:
- lock_replies == 0 (caller already holds the reply lock): behavior unchanged —
no lock/unlock here, _reply_light still receives 0.
- lock_replies == 1: the lock now also covers the lumps; _reply_light is told
not to retake it. Exactly one unlock on the success path and one on the error
path (error_1 falls through to error).
- The lock is per-transaction and a transaction emits few replies, so the added
hold time is negligible. TMCB_LOCAL_RESPONSE and build_res_buf_from_sip_req()
now run under reply_mutex; neither retakes the reply lock today, so this is
safe. If preferred, the callback could be moved out of the locked window.

Reproduction

1. Any b2b_entities B2BUA (or any caller of t_reply_with_body with a
body/extra header) forwarding callee replies to the caller's UAS transaction.
2. Drive a few hundred concurrent calls that answer (200 OK + SDP) with a slice
that CANCELs mid-ring, so caller-side replies glare with the 200 OK on the
same UAS transaction.
3. Without the patch: workers SIGSEGV in add_lump_rpl (data_lump_rpl.c:80),
cores accumulate, the proxy goes down.
4. With the patch: a 150-concurrent-call run, 15% mid-ring cancels, 45 s, produced
zero crashes (vs. hundreds of cores before).

[bogdan-iancu]

Hi @davegreeko2023 , thanks for the report. The t_reply_with_body is a bit weird function by design, degenerating into several problems - see also #3422 . More or the less, because is operating on the SIP message stored in Shm, rather than a pkg SIP request.
Of course, the quick one here will be to enclose the whole function within a critical region, so it will be safe for parallel running. Still this will not solved the other bug. Let me think a bit on this