Change in osmo-ttcn3-hacks[master]: msc: add f_tc_invalid_mgcp_crash
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@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() );
Change in osmo-ttcn3-hacks[master]: msc: add f_tc_invalid_mgcp_crash
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15942 ) Change subject: msc: add f_tc_invalid_mgcp_crash .. Patch Set 1: Code-Review+2 -- 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: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Comment-Date: Mon, 04 Nov 2019 11:23:53 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-ttcn3-hacks[master]: msc: add f_tc_invalid_mgcp_crash
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/15942 ) Change subject: msc: add f_tc_invalid_mgcp_crash .. Patch Set 1: Code-Review+1 -- 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: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Comment-Date: Mon, 04 Nov 2019 10:39:44 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-ttcn3-hacks[master]: msc: add f_tc_invalid_mgcp_crash
neels has uploaded this change for review. (
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(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks
refs/changes/42/15942/1
diff --git a/msc/BSC_ConnectionHandler.ttcn b/msc/BSC_ConnectionHandler.ttcn
index 853d1e5..092906d 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@mgw",
use_osmux := false,
@@ -1210,6 +1212,8 @@
ts_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
