[MERGED] osmo-sip-connector[master]: mncc: Fix use after free on mncc socket disconnection

[Holger Freyther]

Holger Freyther has submitted this change and it was merged.

Change subject: mncc: Fix use after free on mncc socket disconnection
......................................................................


mncc: Fix use after free on mncc socket disconnection

When the MNCC socket breaks down we would release all callds but when
there is no remote call the call would be released before

	if (call->remote)
		...

is being executed leading to a use after free. Fix it by copying the
legs first and assuming the call will be gone after that.

==3618== Invalid read of size 4
==3618==    at 0x804A18A: app_mncc_disconnected (app.c:49)
==3618==    by 0x804B52D: close_connection (mncc.c:255)
==3618==    by 0x804BCFA: mncc_rtp_send.constprop.13 (mncc.c:145)
==3618==    by 0x804CC86: check_setup (mncc.c:435)
==3618==    by 0x804CC86: mncc_data (mncc.c:795)
==3618==    by 0x42FCF94: osmo_fd_disp_fds (select.c:167)
==3618==    by 0x804D1F2: evpoll (evpoll.c:92)
==3618==    by 0x4205053: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4200.1)
==3618==    by 0x4205478: g_main_loop_run (in /lib/i386-linux-gnu/libglib-2.0.so.0.4200.1)
==3618==    by 0x8049AA6: main (main.c:171)
==3618==  Address 0x47f3258 is 64 bytes inside a block of size 76 free'd
==3618==    at 0x402A3A8: free (vg_replace_malloc.c:473)
==3618==    by 0x42E7FD1: ??? (in /usr/lib/i386-linux-gnu/libtalloc.so.2.1.5)
==3618==    by 0x804A3FD: call_leg_release (call.c:87)
==3618==    by 0x804A186: app_mncc_disconnected (app.c:48)
==3618==    by 0x804B52D: close_connection (mncc.c:255)
==3618==    by 0x804BCFA: mncc_rtp_send.constprop.13 (mncc.c:145)
==3618==    by 0x804CC86: check_setup (mncc.c:435)
==3618==    by 0x804CC86: mncc_data (mncc.c:795)
==3618==    by 0x42FCF94: osmo_fd_disp_fds (select.c:167)
==3618==    by 0x804D1F2: evpoll (evpoll.c:92)
==3618==    by 0x4205053: ??? (in /lib/i386-linux-gnu/libglib-2.0.so.0.4200.1)
==3618==    by 0x4205478: g_main_loop_run (in /lib/i386-linux-gnu/libglib-2.0.so.0.4200.1)
==3618==    by 0x8049AA6: main (main.c:171)
==3618==

Change-Id: I1889013ed315f896e4295358f6daf76ce523dc2a
---
M src/app.c
1 file changed, 12 insertions(+), 5 deletions(-)

Approvals:
  Neels Hofmeyr: Looks good to me, but someone else must approve
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/app.c b/src/app.c
index 585b577..97123b8 100644
--- a/src/app.c
+++ b/src/app.c
@@ -29,6 +29,7 @@
 	struct call *call, *tmp;
 
 	llist_for_each_entry_safe(call, tmp, &g_call_list, entry) {
+		struct call_leg *initial, *remote;
 		int has_mncc = 0;
 
 		if (call->initial && call->initial->type == CALL_TYPE_MNCC)
@@ -40,14 +41,20 @@
 			continue;
 
 		/*
-		 * this call has a MNCC component and we will release it.
+		 * this call has a MNCC component and we will release it now.
+		 * There might be no remote so on the release of the initial
+		 * leg the call might be gone. We may not touch call beyond
+		 * that point.
 		 */
 		LOGP(DAPP, LOGL_NOTICE,
 			"Going to release call(%u) due MNCC.\n", call->id);
-		if (call->initial)
-			call->initial->release_call(call->initial);
-		if (call->remote)
-			call->remote->release_call(call->remote);
+		initial = call->initial;
+		remote = call->remote;
+		call = NULL;
+		if (initial)
+			initial->release_call(initial);
+		if (remote)
+			remote->release_call(remote);
 	}
 }
 

-- 
To view, visit https://gerrit.osmocom.org/1973
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I1889013ed315f896e4295358f6daf76ce523dc2a
Gerrit-PatchSet: 2
Gerrit-Project: osmo-sip-connector
Gerrit-Branch: master
Gerrit-Owner: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>