Skip to content

tm: fix shm double-free of script-route refs on shared transactions#4113

Open
kvwake wants to merge 1 commit into
OpenSIPS:masterfrom
kvwake:fix/tm-on_reply-shm-double-free
Open

tm: fix shm double-free of script-route refs on shared transactions#4113
kvwake wants to merge 1 commit into
OpenSIPS:masterfrom
kvwake:fix/tm-on_reply-shm-double-free

Conversation

@kvwake

@kvwake kvwake commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a shared-memory double-free in the tm module that crashes a worker and, on FAST_LOCK builds, locks the entire opensips server until restart. More details in #4112.

t_on_reply(), t_on_negative() (script t_on_failure) and t_on_branch() update a script-route reference stored in the transaction cell with an unlocked read-modify-write:

holder = (!t || t==T_UNDEFINED) ? &goto_on_reply
       : (route_type==BRANCH_ROUTE ? &TM_BRANCH(t,_tm_branch_index).on_reply : &t->on_reply);

if (*holder)
    shm_free(*holder);
*holder = ref ? dup_ref_script_route_in_shm(ref, 0) : NULL;

When these are called from a request route against an already-existing transaction, a retransmitted in-dialog ACK matched via t_check_trans() and processed by multiple UDP workers concurrently. holder points into the shared transaction cell. Two processes read the same pointer and both shm_free() it. The second free trips the allocator's double-free detector, which abort()s while holding the global shm lock. on a FAST_LOCK build the orphaned lock then makes every process spin at 100% CPU and the server stops processing traffic (sockets stay open, nothing is handled) until it is restarted.

Found in 3.4.17, but same code for 3.5/3.6/4.0/master

Solution

Route the three setters through a shared helper (tm_set_script_route_ref()) that swaps the holder pointer atomically under the transaction reply lock and frees the swapped-out (now private) pointer afterwards. The shm alloc/free are kept outside LOCK_REPLIES, so the reply lock is never nested around the global shm lock.

The reply lock is taken only in REQUEST_ROUTE, the sole context that can reach a shared cell field concurrently without already holding the reply lock. FAILURE_ROUTE and, with onreply_avp_mode, ONREPLY_ROUTE already run under LOCK_REPLIES and may call these handlers, so acquiring the non-recursive reply lock there would probably deadlock.

An lock free approach would be to skip the update when the transaction already exists in REQUEST_ROUTE (setting on_reply on an already-created transaction is a no-op). Switch to that if preferred — the current PR keeps existing behavior and only makes the update safe.

Builds clean under gcc and clang with -Werror and passes the unit-test suite (make test). Let me know if I missed any testing that should be done prior to submitting a PR.

Compatibility

No script- or API-level changes; no configuration or migration required. Behavior is unchanged except that concurrent updates of the shared route-ref field are now serialized.

Closing issues

closes #4112

t_on_reply()/t_on_negative()/t_on_branch() did an unlocked
"if (*holder) shm_free(*holder); *holder = dup(...)" on a route-ref field
of the shared transaction cell. From a request route against an existing
transaction (e.g. a retransmitted in-dialog ACK handled by two UDP
workers concurrently), both read the same pointer and free it; the second
shm_free() aborts while holding the global shm lock, wedging every
process (FAST_LOCK spin) until restart.

Swap the holder atomically under the transaction reply lock, freeing the
old pointer outside the lock. Take the lock only in REQUEST_ROUTE, since
FAILURE_ROUTE and (with onreply_avp_mode) ONREPLY_ROUTE already hold
LOCK_REPLIES and may re-arm these handlers.

Refs OpenSIPS#4112
@kvwake
kvwake marked this pull request as ready for review July 16, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CRASH] tm: shm double-free in t_on_reply()/t_on_failure() on concurrent in-dialog ACKs

1 participant