Change in osmo-ttcn3-hacks[master]: msc: add f_tc_invalid_mgcp_crash

[laforge]

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15942 )

Change subject: msc: add f_tc_invalid_mgcp_crash
......................................................................

msc: add f_tc_invalid_mgcp_crash

Make sure that osmo-msc doesn't crash if a successful CRCX response contains an
invalid IP address.

Originally/recently, osmo-msc did not validate the IP addresses at all. In an
intermediate patch I added error handling, releasing the call. That uncovered a
use-after-free problem in libosmo-mgcp-client. This problem is fixed by
osmo_fsm_set_dealloc_ctx() and an osmo-mgw fix (see
I7df2e9202b04e7ca7366bb0a8ec53cf3bb14faf3 in osmo-mgw).

Add this test to make sure the crash is not re-introduced.

Change-Id: I0c76b0a7a33a96a39a242ecd387ba3769161cf7a
---
M msc/BSC_ConnectionHandler.ttcn
M msc/MSC_Tests.ttcn
2 files changed, 40 insertions(+), 0 deletions(-)

Approvals:
  laforge: Looks good to me, but someone else must approve
  pespin: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn
index a1c8bd3..0846c04 100644
--- a/msc/BSC_ConnectionHandler.ttcn
+++ b/msc/BSC_ConnectionHandler.ttcn
@@ -711,6 +711,7 @@
 							   (f_mt_call and f_mt_call) */
 	boolean stop_after_cc_setup,			/* Special case: stop call establish after CC Setup */
 	boolean ran_clear_when_alerting,		/* Special case: send Clear upon CC Alerting */
+	boolean expect_release,				/* Special case: expect call establish to cause direct CC Rel */
 
 	MgcpCallId mgcp_call_id optional,		/* MGCP Call ID; CallAgent allocated */
 	MgcpEndpoint mgcp_ep optional			/* MGCP Endpoint, CallAgent or MGW allocated */,
@@ -749,6 +750,7 @@
 	mgw_drop_dlcx := false,
 	stop_after_cc_setup := false,
 	ran_clear_when_alerting := false,
+	expect_release := false,
 	mgcp_call_id := omit,
 	mgcp_ep := "rtpbridge/1 at mgw",
 	use_osmux := false,
@@ -1210,6 +1212,8 @@
 		tr_BSSMAP_IE_AoIP_TLA4(f_inet_addr(cpars.mgw_conn_1.mgw_rtp_ip), ?);
 
 	var default mdcx := activate(as_optional_mgcp_mdcx(cpars.mgw_conn_2.mgw_rtp_ip, cpars.mgw_conn_2.mgw_rtp_port));
+	var boolean got_mncc_setup_compl_ind := false;
+	var boolean got_cc_connect := false;
 
 	interleave {
 	[] MNCC.receive(tr_MNCC_SETUP_ind(?, tr_MNCC_number(hex2str(cpars.called_party)))) -> value mncc {
@@ -1347,15 +1351,27 @@
 
 	[] MNCC.receive(tr_MNCC_SETUP_COMPL_ind(?)) -> value mncc {
 		log("f_mo_call_establish 8: rx MNCC SETUP COMPLETE ind");
+		got_mncc_setup_compl_ind := true;
+		if (not cpars.expect_release and got_cc_connect) {
+			break;
+		}
 		}
 
 	[] BSSAP.receive(tr_PDU_DTAP_MT(tr_ML3_MT_CC_CONNECT(cpars.transaction_id))) {
 		log("f_mo_call_establish 10: rx CC CONNECT");
 		BSSAP.send(ts_PDU_DTAP_MO(ts_ML3_MO_CC_CONNECT_ACK(cpars.transaction_id)));
+		got_cc_connect := true;
+		if (not cpars.expect_release and got_mncc_setup_compl_ind) {
+			break;
+		}
 		}
 
 	[] BSSAP.receive(tr_PDU_DTAP_MT(tr_ML3_MT_CC_RELEASE(cpars.transaction_id))) {
 		log("f_mo_call_establish 11: rx CC RELEASE");
+		if (not cpars.expect_release) {
+			setverdict(fail, "Got unexpected CC Release");
+			mtc.stop;
+		}
 		f_expect_clear();
 		break;
 		}
diff --git a/msc/MSC_Tests.ttcn b/msc/MSC_Tests.ttcn
index 4ef592f..480ec96 100644
--- a/msc/MSC_Tests.ttcn
+++ b/msc/MSC_Tests.ttcn
@@ -5662,6 +5662,29 @@
 	vc_conn.done;
 }
 
+friend function f_tc_invalid_mgcp_crash(charstring id, BSC_ConnHdlrPars pars) runs on BSC_ConnHdlr {
+	f_init_handler(pars);
+	var CallParameters cpars := valueof(t_CallParams('12345'H, 0));
+
+	/* Set invalid IP address so that osmo-msc discards the rtp_stream and MGCP endpoint FSM instances in the middle
+	 * of successful MGCP response dispatch. If things aren't safeguarded, the on_success() in osmo_mgcpc_ep_fsm
+	 * will cause a use-after-free after that event dispatch. */
+	cpars.mgw_conn_1.mgw_rtp_ip := "0.0.0.0";
+	cpars.mgw_conn_2.mgw_rtp_ip := "0.0.0.0";
+	cpars.rtp_sdp_format := "FOO/8000";
+	cpars.expect_release := true;
+
+	f_perform_lu();
+	f_mo_call_establish(cpars);
+}
+testcase TC_invalid_mgcp_crash() runs on MTC_CT {
+	var BSC_ConnHdlr vc_conn;
+	f_init();
+
+	vc_conn := f_start_handler(refers(f_tc_invalid_mgcp_crash), 7);
+	vc_conn.done;
+}
+
 control {
 	execute( TC_cr_before_reset() );
 	execute( TC_lu_imsi_noauth_tmsi() );
@@ -5792,6 +5815,7 @@
 	if (mp_enable_osmux_test) {
 		execute( TC_lu_and_mt_call_osmux() );
 	}
+	execute( TC_invalid_mgcp_crash() );
 }
 
 

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15942
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I0c76b0a7a33a96a39a242ecd387ba3769161cf7a
Gerrit-Change-Number: 15942
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191123/0a7cefa9/attachment.htm>