[Asterisk-code-review] res_pjsip_mwi: potential double unref, and potential unwanted double ... (...asterisk[13])

Kevin Harwell asteriskteam at digium.com
Thu Oct 10 15:29:51 CDT 2019


Kevin Harwell has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/13045


Change subject: res_pjsip_mwi: potential double unref, and potential unwanted double link
......................................................................

res_pjsip_mwi: potential double unref, and potential unwanted double link

When creating an unsolicited MWI aggregate subscription it was possible for
the subscription object to be double unref'ed. This patch removes the explicit
unref as it is not needed since the RAII_VAR will handle it at function end.

Less concerning there was also a bug that could potentially allow the aggregate
subscription object to be added to the unsolicited container twice. This patch
ensures it is added only once.

ASTERISK-28575

Change-Id: I9ccfdb5ea788bc0c3618db183aae235e53c12763
---
M res/res_pjsip_mwi.c
1 file changed, 11 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/45/13045/1

diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c
index e14860e..e5d342c 100644
--- a/res/res_pjsip_mwi.c
+++ b/res/res_pjsip_mwi.c
@@ -1223,6 +1223,13 @@
 			if (!aggregate_sub) {
 				return 0; /* No MWI aggregation for you */
 			}
+
+			/*
+			 * Just in case we somehow get in the position of recreating with no previous
+			 * aggregate object, set recreate to false here in order to allow the new
+			 * object to be linked into the container below
+			 */
+			recreate = 0;
 		}
 	}
 
@@ -1266,13 +1273,13 @@
 
 	if (aggregate_sub) {
 		if (ao2_container_count(aggregate_sub->stasis_subs)) {
-			ao2_link_flags(unsolicited_mwi, aggregate_sub, OBJ_NOLOCK);
+			/* Only link if we're dealing with a new aggregate object */
+			if (!recreate) {
+				ao2_link_flags(unsolicited_mwi, aggregate_sub, OBJ_NOLOCK);
+			}
 			if (send_now && sub_added) {
 				send_notify(aggregate_sub, NULL, 0);
 			}
-		} else {
-			/* No stasis subscriptions then no MWI data to aggregate */
-			ao2_ref(aggregate_sub, -1);
 		}
 	}
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I9ccfdb5ea788bc0c3618db183aae235e53c12763
Gerrit-Change-Number: 13045
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191010/c0978f99/attachment-0001.html>


More information about the asterisk-code-review mailing list