[Asterisk-code-review] res/res pjsip: Fix off nominal crash with requests that fail... (asterisk[master])

[Matt Jordan]

Matt Jordan has submitted this change and it was merged.

Change subject: res/res_pjsip: Fix off nominal crash with requests that fail and have a timer
......................................................................


res/res_pjsip: Fix off nominal crash with requests that fail and have a timer

When a request is sent using pjsip_endpt_send_request and fails, a condition
exists where the request wrapper, which is an AO2 object, may be de-ref'd
more times than it should. This occurs when the request's callback is called,
and, in the callback, the timer on the PJSIP heap is cancelled. When that
occurs, the request wrapper's lifetime is decremented. When
pjsip_endpt_send_request fails, we unilaterally decrement the lifetime of
the request wrapper again, even though we've already cancelled the reference
associated with the timer.

This patch checks the return result of pj_timer_heap_cancel_if_active before
removing the reference associated with the timer. We now only decrement it
in this case if a timer is cancelled as a result of the function call.

Change-Id: I21332343a1a019c1117076f9bf2df27be2850102
---
M res/res_pjsip.c
1 file changed, 4 insertions(+), 2 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Matt Jordan: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 9d0540d..a4748d2 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -3151,9 +3151,11 @@
 		char errmsg[PJ_ERR_MSG_SIZE];
 
 		if (timeout > 0) {
-			pj_timer_heap_cancel_if_active(pjsip_endpt_get_timer_heap(endpt),
+			int timers_cancelled = pj_timer_heap_cancel_if_active(pjsip_endpt_get_timer_heap(endpt),
 				req_wrapper->timeout_timer, TIMER_INACTIVE);
-			ao2_ref(req_wrapper, -1);
+			if (timers_cancelled > 0) {
+				ao2_ref(req_wrapper, -1);
+			}
 		}
 
 		/* Complain of failure to send the request. */

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I21332343a1a019c1117076f9bf2df27be2850102
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>