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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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
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