[Asterisk-code-review] res/res pjsip session: Check for presence of an active negot... (asterisk[13])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: res/res_pjsip_session: Check for presence of an active negotiator
......................................................................


res/res_pjsip_session: Check for presence of an active negotiator

It is possible in a hypothetical situation for a session refresh to be
invoked on a PJSIP when the negotiatior on the INVITE session has not
yet been established. While this shouldn't occur with existing uses of
ast_sip_session_refresh, the crashes that occur due to improperly
calling PJSIP functions that expect a non-NULL negotiatior are
avoidable. PJSIP will create the negotiator in pjsip_inv_reinvite; this
means that simply checking for the presence of the negotiator before
passing it to other PJSIP functions that use it is allowable. As such,
this patch adds checks for the presence of the negotiator before calling
PJSIP functions that assume it is non-NULL.

Change-Id: I1028323e7e01b0a531865e5412a71b6f6ec4276d
---
M res/res_pjsip_session.c
1 file changed, 10 insertions(+), 6 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved
  Richard Mudgett: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 961926b..4290f68 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -792,12 +792,14 @@
 static pjmedia_sdp_session *generate_session_refresh_sdp(struct ast_sip_session *session)
 {
 	pjsip_inv_session *inv_session = session->inv_session;
-	const pjmedia_sdp_session *previous_sdp;
+	const pjmedia_sdp_session *previous_sdp = NULL;
 
-	if (pjmedia_sdp_neg_was_answer_remote(inv_session->neg)) {
-		pjmedia_sdp_neg_get_active_remote(inv_session->neg, &previous_sdp);
-	} else {
-		pjmedia_sdp_neg_get_active_local(inv_session->neg, &previous_sdp);
+	if (inv_session->neg) {
+		if (pjmedia_sdp_neg_was_answer_remote(inv_session->neg)) {
+			pjmedia_sdp_neg_get_active_remote(inv_session->neg, &previous_sdp);
+		} else {
+			pjmedia_sdp_neg_get_active_local(inv_session->neg, &previous_sdp);
+		}
 	}
 	return create_local_sdp(inv_session, session, previous_sdp);
 }
@@ -917,7 +919,9 @@
 
 	if (generate_new_sdp) {
 		/* SDP can only be generated if current negotiation has already completed */
-		if (pjmedia_sdp_neg_get_state(inv_session->neg) != PJMEDIA_SDP_NEG_STATE_DONE) {
+		if (inv_session->neg
+			&& pjmedia_sdp_neg_get_state(inv_session->neg)
+				!= PJMEDIA_SDP_NEG_STATE_DONE) {
 			ast_debug(3, "Delay session refresh with new SDP to %s because SDP negotiation is not yet done...\n",
 				ast_sorcery_object_get_id(session->endpoint));
 			return delay_request(session, on_request_creation, on_sdp_creation,

-- 
To view, visit https://gerrit.asterisk.org/3111
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I1028323e7e01b0a531865e5412a71b6f6ec4276d
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>