<p>fixeria has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-msc/+/14463">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">libmsc/gsm_09_11.c: do not abuse LOG_TRANS() and early trans allocation<br><br>In case of network-originated SS/USSD session establishment, we<br>need to verify the received GSUP PROC_SS_REQ message and make<br>sure that all mandatory IEs are present.<br><br>There is no sensible need to allocate a new transaction before<br>doing all the checks. This complicates the code.<br><br>Change-Id: I4e027b19e8065a39324a1647957cef4066b82ce7<br>---<br>M src/libmsc/gsm_09_11.c<br>M tests/msc_vlr/msc_vlr_test_ss.err<br>2 files changed, 22 insertions(+), 31 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/63/14463/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/libmsc/gsm_09_11.c b/src/libmsc/gsm_09_11.c</span><br><span>index 4a7c348..9acff0c 100644</span><br><span>--- a/src/libmsc/gsm_09_11.c</span><br><span>+++ b/src/libmsc/gsm_09_11.c</span><br><span>@@ -317,53 +317,44 @@</span><br><span> struct gsm_trans *trans, *transt;</span><br><span> int tid;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Allocate transaction first, for log context */</span><br><span style="color: hsl(0, 100%, 40%);">- trans = trans_alloc(net, vsub, TRANS_USSD,</span><br><span style="color: hsl(0, 100%, 40%);">- TRANS_ID_UNASSIGNED, gsup_msg->session_id);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (!trans) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_TRANS(trans, LOGL_ERROR, " -> No memory for trans\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> if (gsup_msg->session_state != OSMO_GSUP_SESSION_STATE_BEGIN) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_TRANS(trans, LOGL_ERROR, "Received non-BEGIN message "</span><br><span style="color: hsl(0, 100%, 40%);">- "for non-existing transaction\n");</span><br><span style="color: hsl(0, 100%, 40%);">- trans_free(trans);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DSS, LOGL_ERROR, "Received non-BEGIN message for non-existing transaction\n");</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DSS, LOGL_DEBUG, "Establishing a network-originated session (id=0x%x) "</span><br><span style="color: hsl(120, 100%, 40%);">+ "with subscriber %s\n", gsup_msg->session_id,</span><br><span style="color: hsl(120, 100%, 40%);">+ vlr_subscr_name(vsub));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (!gsup_msg->ss_info || gsup_msg->ss_info_len < 2) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_TRANS(trans, LOGL_ERROR, "Missing mandatory Facility IE\n");</span><br><span style="color: hsl(0, 100%, 40%);">- trans_free(trans);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DSS, LOGL_ERROR, "Missing mandatory Facility IE\n");</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span> /* If subscriber is not "attached" */</span><br><span> if (!vsub->cgi.lai.lac) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_TRANS(trans, LOGL_ERROR, "Network-originated session "</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DSS, LOGL_ERROR, "Network-originated session "</span><br><span> "rejected - subscriber is not attached\n");</span><br><span style="color: hsl(0, 100%, 40%);">- trans_free(trans);</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- LOG_TRANS(trans, LOGL_DEBUG, "Establishing network-originated session\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Obtain an unused transaction ID */</span><br><span style="color: hsl(120, 100%, 40%);">+ tid = trans_assign_trans_id(net, vsub, TRANS_USSD);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (tid < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DSS, LOGL_ERROR, "No free transaction ID\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Allocate a new NCSS transaction */</span><br><span style="color: hsl(120, 100%, 40%);">+ trans = trans_alloc(net, vsub, TRANS_USSD, tid, gsup_msg->session_id);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!trans) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DSS, LOGL_ERROR, " -> No memory for trans\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> /* Count active NC SS/USSD sessions */</span><br><span> osmo_counter_inc(net->active_nc_ss);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Assign transaction ID */</span><br><span style="color: hsl(0, 100%, 40%);">- tid = trans_assign_trans_id(trans->net, trans->vsub, TRANS_USSD);</span><br><span style="color: hsl(0, 100%, 40%);">- if (tid < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_TRANS(trans, LOGL_ERROR, "No free transaction ID\n");</span><br><span style="color: hsl(0, 100%, 40%);">- /* TODO: inform HLR about this */</span><br><span style="color: hsl(0, 100%, 40%);">- /* TODO: release connection with subscriber */</span><br><span style="color: hsl(0, 100%, 40%);">- trans->callref = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- trans_free(trans);</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- trans->transaction_id = tid;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /* Init inactivity timer */</span><br><span> osmo_timer_setup(&trans->ss.timer_guard,</span><br><span> ncss_session_timeout_handler, trans);</span><br><span>diff --git a/tests/msc_vlr/msc_vlr_test_ss.err b/tests/msc_vlr/msc_vlr_test_ss.err</span><br><span>index 1199cf1..e058e37 100644</span><br><span>--- a/tests/msc_vlr/msc_vlr_test_ss.err</span><br><span>+++ b/tests/msc_vlr/msc_vlr_test_ss.err</span><br><span>@@ -367,9 +367,9 @@</span><br><span> llist_count(&vsub->cs.requests) == 0</span><br><span> <-- GSUP rx OSMO_GSUP_MSGT_PROC_SS_REQUEST: 20010809710000004026f03004200001013101013515a11302010102013b300b04010f0406aa510c061b010a0103</span><br><span> DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + gsm0911_gsup_rx: now used by 3 (attached,_test_ss_ussd_no,gsm0911_gsup_rx)</span><br><span style="color: hsl(120, 100%, 40%);">+DSS Establishing a network-originated session (id=0x20000101) with subscriber IMSI-901700000004620:MSISDN-46071</span><br><span> DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + NCSS: now used by 4 (attached,_test_ss_ussd_no,gsm0911_gsup_rx,NCSS)</span><br><span style="color: hsl(0, 100%, 40%);">-DSS trans(NCSS IMSI-901700000004620:MSISDN-46071 callref-0x20000101 tid-255) New transaction</span><br><span style="color: hsl(0, 100%, 40%);">-DSS trans(NCSS IMSI-901700000004620:MSISDN-46071 callref-0x20000101 tid-255) Establishing network-originated session</span><br><span style="color: hsl(120, 100%, 40%);">+DSS trans(NCSS IMSI-901700000004620:MSISDN-46071 callref-0x20000101 tid-0) New transaction</span><br><span> DSS trans(NCSS IMSI-901700000004620:MSISDN-46071 callref-0x20000101 tid-0) Triggering Paging Request</span><br><span> DPAG Paging: IMSI-901700000004620:MSISDN-46071 for GSM 09.11 SS/USSD: Starting paging</span><br><span> paging request (SIGNALLING_HIGH_PRIO) to IMSI-901700000004620:MSISDN-46071 on GERAN-A</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-msc/+/14463">change 14463</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-msc/+/14463"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-msc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I4e027b19e8065a39324a1647957cef4066b82ce7 </div>
<div style="display:none"> Gerrit-Change-Number: 14463 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>