[Asterisk-code-review] pjsip: Make modify_local_offer2 tolerate previous failed SDP. (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15434 )

Change subject: pjsip: Make modify_local_offer2 tolerate previous failed SDP.
......................................................................

pjsip: Make modify_local_offer2 tolerate previous failed SDP.

If a remote side is broken and sends an SDP that can not be
negotiated the call will be torn down but there is a window
where a second 183 Session Progress or 200 OK that is forked
can be received that also attempts to negotiate SDP. Since
the code marked the SDP negotiation as being done and complete
prior to this it assumes that there is an active local and remote
SDP which it can modify, while in fact there is not as the SDP
did not successfully negotiate. Since there is no local or remote
SDP a crash occurs.

This patch changes the pjmedia_sdp_neg_modify_local_offer2
function to no longer assume that a previous SDP negotiation
was successful.

ASTERISK-29196

Change-Id: I22de45916d3b05fdc2a67da92b3a38271ee5949e
---
A third-party/pjproject/patches/0080-fix-sdp-neg-modify-local-offer.patch
1 file changed, 33 insertions(+), 0 deletions(-)

Approvals:
  Friendly Automation: Verified
  Joshua Colp: Looks good to me, approved; Approved for Submit



diff --git a/third-party/pjproject/patches/0080-fix-sdp-neg-modify-local-offer.patch b/third-party/pjproject/patches/0080-fix-sdp-neg-modify-local-offer.patch
new file mode 100644
index 0000000..c27a489
--- /dev/null
+++ b/third-party/pjproject/patches/0080-fix-sdp-neg-modify-local-offer.patch
@@ -0,0 +1,33 @@
+diff --git a/pjmedia/src/pjmedia/sdp_neg.c b/pjmedia/src/pjmedia/sdp_neg.c
+index 3b85b4273..a14009662 100644
+--- a/pjmedia/src/pjmedia/sdp_neg.c
++++ b/pjmedia/src/pjmedia/sdp_neg.c
+@@ -304,7 +304,6 @@ PJ_DEF(pj_status_t) pjmedia_sdp_neg_modify_local_offer2(
+ {
+     pjmedia_sdp_session *new_offer;
+     pjmedia_sdp_session *old_offer;
+-    char media_used[PJMEDIA_MAX_SDP_MEDIA];
+     unsigned oi; /* old offer media index */
+     pj_status_t status;
+ 
+@@ -323,8 +322,19 @@ PJ_DEF(pj_status_t) pjmedia_sdp_neg_modify_local_offer2(
+     /* Change state to STATE_LOCAL_OFFER */
+     neg->state = PJMEDIA_SDP_NEG_STATE_LOCAL_OFFER;
+ 
++    /* When there is no active local SDP in state PJMEDIA_SDP_NEG_STATE_DONE,
++     * it means that the previous initial SDP nego must have been failed,
++     * so we'll just set the local SDP offer here.
++     */
++    if (!neg->active_local_sdp) {
++	neg->initial_sdp_tmp = NULL;
++	neg->initial_sdp = pjmedia_sdp_session_clone(pool, local);
++	neg->neg_local_sdp = pjmedia_sdp_session_clone(pool, local);
++
++	return PJ_SUCCESS;
++    }
++
+     /* Init vars */
+-    pj_bzero(media_used, sizeof(media_used));
+     old_offer = neg->active_local_sdp;
+     new_offer = pjmedia_sdp_session_clone(pool, local);
+ 

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15434
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I22de45916d3b05fdc2a67da92b3a38271ee5949e
Gerrit-Change-Number: 15434
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210218/adfa68ce/attachment-0001.html>