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() ); 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 <nhofm...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-MessageType: merged