Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update cloned msg under lock to avoid races with branch injection #3528

Merged
merged 1 commit into from
Mar 3, 2025

Conversation

vladpaiu
Copy link
Member

@vladpaiu vladpaiu commented Nov 26, 2024

Summary
Aims to fix double free crash happening when a branch gets injected into a transaction which has not relayed yet.

Details

  1. process Y creates the transaction and calls t_wait_for_new_branches() ( either directly or via lookup() )
  2. procesX tries to inject a new branch and makes a clone of t.uas.request https://github.com/OpenSIPS/opensips/blob/master/modules/tm/t_fwd.c#L1064
  3. proces Y calls t_relay() and updates t.uas.request https://github.com/OpenSIPS/opensips/blob/master/modules/tm/tm.c#L1339, frees old add_rm ( along other old fields ) https://github.com/OpenSIPS/opensips/blob/master/modules/tm/sip_msg.c#L1286
  4. procesX tries to free it's fake req, https://github.com/OpenSIPS/opensips/blob/master/modules/tm/t_msgbuilder.h#L403 ,finds a different pointer than the one in t.uas.request, ending in a double free

Solution
When doing t_relay(), update the UAS request under lock, to avoid these race conditions

Compatibility
Should be backwards compatible

@vladpaiu vladpaiu added the bug label Nov 26, 2024
@vladpaiu vladpaiu added this to the 3.6-dev milestone Nov 26, 2024
@vladpaiu vladpaiu requested review from akuriakose-sorenson and removed request for akuriakose-sorenson November 27, 2024 18:59
@bogdan-iancu bogdan-iancu merged commit b868e00 into OpenSIPS:master Mar 3, 2025
51 checks passed
@bogdan-iancu
Copy link
Member

Good catch @vladpaiu , thanks for the report and fix

bogdan-iancu added a commit that referenced this pull request Mar 3, 2025
fix: update cloned msg under lock to avoid races with branch injection
(cherry picked from commit b868e00)
bogdan-iancu added a commit that referenced this pull request Mar 3, 2025
fix: update cloned msg under lock to avoid races with branch injection
(cherry picked from commit b868e00)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants