[Asterisk-code-review] AST-2018-005: Fix tdata leaks when calling pjsip endpt send ... (asterisk[13.19])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/8325 )

Change subject: AST-2018-005: Fix tdata leaks when calling pjsip_endpt_send_response(2)
......................................................................

AST-2018-005: Fix tdata leaks when calling pjsip_endpt_send_response(2)

pjsip_distributor:
   authenticate() creates a tdata and uses it to send a challenge or
   failure response.  When pjsip_endpt_send_response2() succeeds, it
   automatically decrements the tdata ref count but when it fails, it
   doesn't.  Since we weren't checking for a return status, we weren't
   decrementing the count ourselves on error and were therefore leaking
   tdatas.

res_pjsip_session:
   session_reinvite_on_rx_request wasn't decrementing the ref count
   if an error happened while sending a 491 response.
   pre_session_setup wasn't decrementing the ref count if
   while sending an error after a pjsip_inv_verify_request failure.

res_pjsip:
   ast_sip_send_response wasn't decrementing the ref count on error.

ASTERISK-27618
Reported By: Sandro Gauci

Change-Id: Iab33a6c7b6fba96148ed465b690ba8534ac961bf
---
M res/res_pjsip.c
M res/res_pjsip/pjsip_distributor.c
M res/res_pjsip_session.c
3 files changed, 20 insertions(+), 6 deletions(-)

Approvals:
  Jenkins2: Verified
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 392f9a6..0bd5ceb 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -4402,9 +4402,15 @@
 
 int ast_sip_send_response(pjsip_response_addr *res_addr, pjsip_tx_data *tdata, struct ast_sip_endpoint *sip_endpoint)
 {
-	supplement_outgoing_response(tdata, sip_endpoint);
+	pj_status_t status;
 
-	return pjsip_endpt_send_response(ast_sip_get_pjsip_endpoint(), res_addr, tdata, NULL, NULL);
+	supplement_outgoing_response(tdata, sip_endpoint);
+	status = pjsip_endpt_send_response(ast_sip_get_pjsip_endpoint(), res_addr, tdata, NULL, NULL);
+	if (status != PJ_SUCCESS) {
+		pjsip_tx_data_dec_ref(tdata);
+	}
+
+	return status == PJ_SUCCESS ? 0 : -1;
 }
 
 int ast_sip_send_stateful_response(pjsip_rx_data *rdata, pjsip_tx_data *tdata, struct ast_sip_endpoint *sip_endpoint)
diff --git a/res/res_pjsip/pjsip_distributor.c b/res/res_pjsip/pjsip_distributor.c
index 591d7fe..e056b60 100644
--- a/res/res_pjsip/pjsip_distributor.c
+++ b/res/res_pjsip/pjsip_distributor.c
@@ -844,7 +844,9 @@
 		case AST_SIP_AUTHENTICATION_CHALLENGE:
 			/* Send the 401 we created for them */
 			ast_sip_report_auth_challenge_sent(endpoint, rdata, tdata);
-			pjsip_endpt_send_response2(ast_sip_get_pjsip_endpoint(), rdata, tdata, NULL, NULL);
+			if (pjsip_endpt_send_response2(ast_sip_get_pjsip_endpoint(), rdata, tdata, NULL, NULL) != PJ_SUCCESS) {
+				pjsip_tx_data_dec_ref(tdata);
+			}
 			return PJ_TRUE;
 		case AST_SIP_AUTHENTICATION_SUCCESS:
 			/* See note in endpoint_lookup about not holding an unnecessary write lock */
@@ -857,7 +859,9 @@
 		case AST_SIP_AUTHENTICATION_FAILED:
 			log_failed_request(rdata, "Failed to authenticate", 0, 0);
 			ast_sip_report_auth_failed_challenge_response(endpoint, rdata);
-			pjsip_endpt_send_response2(ast_sip_get_pjsip_endpoint(), rdata, tdata, NULL, NULL);
+			if (pjsip_endpt_send_response2(ast_sip_get_pjsip_endpoint(), rdata, tdata, NULL, NULL) != PJ_SUCCESS) {
+				pjsip_tx_data_dec_ref(tdata);
+			}
 			return PJ_TRUE;
 		case AST_SIP_AUTHENTICATION_ERROR:
 			log_failed_request(rdata, "Error to authenticate", 0, 0);
diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index bb53dad..83cf0a8 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -1083,7 +1083,9 @@
 
 		/* Otherwise this is a new re-invite, so reject it */
 		if (pjsip_dlg_create_response(dlg, rdata, 491, NULL, &tdata) == PJ_SUCCESS) {
-			pjsip_endpt_send_response2(ast_sip_get_pjsip_endpoint(), rdata, tdata, NULL, NULL);
+			if (pjsip_endpt_send_response2(ast_sip_get_pjsip_endpoint(), rdata, tdata, NULL, NULL) != PJ_SUCCESS) {
+				pjsip_tx_data_dec_ref(tdata);
+			}
 		}
 
 		return PJ_TRUE;
@@ -2051,7 +2053,9 @@
 
 	if (pjsip_inv_verify_request(rdata, &options, NULL, NULL, ast_sip_get_pjsip_endpoint(), &tdata) != PJ_SUCCESS) {
 		if (tdata) {
-			pjsip_endpt_send_response2(ast_sip_get_pjsip_endpoint(), rdata, tdata, NULL, NULL);
+			if (pjsip_endpt_send_response2(ast_sip_get_pjsip_endpoint(), rdata, tdata, NULL, NULL) != PJ_SUCCESS) {
+				pjsip_tx_data_dec_ref(tdata);
+			}
 		} else {
 			pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);
 		}

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

Gerrit-Project: asterisk
Gerrit-Branch: 13.19
Gerrit-MessageType: merged
Gerrit-Change-Id: Iab33a6c7b6fba96148ed465b690ba8534ac961bf
Gerrit-Change-Number: 8325
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180221/922a8e2c/attachment-0001.html>