<p>Vadim Yanitskiy <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/12628">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Harald Welte: Looks good to me, approved
Max: Looks good to me, but someone else must approve
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">libmsc/gsm_04_11.c: introduce and use gsm411_assign_sm_rp_mr()<br><br>Initially, it was assumed that if there is no active RAN connection,<br>we can just start counting from 0x00, as there are no other SMS<br>related transactions, and transaction itself is allocated using<br>talloc_zero(). Until now it was looking good, but...<br><br>As soon as we establish RAN connection with subscriber, we already<br>have a transaction with SM-RP-MR 0x00, but conn->next_rp_ref also<br>remains 0x00 - it isn't being increased!<br><br>It means that we can face a SM-RP-MR conflict (or collision) if<br>another MT SMS would arrive to the MSC (from SMSC over GSUP)<br>when this transaction is still active, i.e. the first SMS is<br>still being sent, because conn->next_rp_ref++ would<br>return 0x00 again.<br><br>Moreover, there might be already a MO SMS transaction, and using<br>the conn->next_rp_ref counter wouldn't prevent us from having<br>duplicate SM-RP-MR value.<br><br>Let's get rid of this per-connection counter, and introduce a<br>function instead, that would iterate over existing transactions<br>and look for an unused SM-RP-MR value.<br><br>This change makes the following test cases pass:<br><br> - TC_gsup_mt_sms_rp_mr,<br> - TC_gsup_mo_mt_sms_rp_mr.<br><br>Discovered by: Neels Hofmeyr<br>Related Change-Id: (TTCN) I3a52d44f4abde9b6b471b9108c1cee905884c9bc<br>Related Change-Id: (TTCN) I17cbbaa64d9bce770f985588e93cd3eecd732120<br>Change-Id: Ife6d954c46b7d8348a4221ab677d0355eb3ee7ac<br>---<br>M include/osmocom/msc/ran_conn.h<br>M src/libmsc/gsm_04_11.c<br>2 files changed, 29 insertions(+), 5 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/msc/ran_conn.h b/include/osmocom/msc/ran_conn.h</span><br><span>index bec7201..affebc8 100644</span><br><span>--- a/include/osmocom/msc/ran_conn.h</span><br><span>+++ b/include/osmocom/msc/ran_conn.h</span><br><span>@@ -102,8 +102,6 @@</span><br><span> </span><br><span> /* LU expiration handling */</span><br><span> uint8_t expire_timer_stopped;</span><br><span style="color: hsl(0, 100%, 40%);">- /* SMS helpers for libmsc */</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t next_rp_ref;</span><br><span> </span><br><span> /* Are we part of a special "silent" call */</span><br><span> int silent_call;</span><br><span>diff --git a/src/libmsc/gsm_04_11.c b/src/libmsc/gsm_04_11.c</span><br><span>index 2f39b06..1edf2d4 100644</span><br><span>--- a/src/libmsc/gsm_04_11.c</span><br><span>+++ b/src/libmsc/gsm_04_11.c</span><br><span>@@ -1028,6 +1028,30 @@</span><br><span> return trans;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* Assigns an (unused) SM-RP-MR value to a given transaction */</span><br><span style="color: hsl(120, 100%, 40%);">+static int gsm411_assign_sm_rp_mr(struct gsm_trans *trans)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t mr;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* After allocation a given transaction has zero-initialized</span><br><span style="color: hsl(120, 100%, 40%);">+ * SM-RP-MR value, so trans_find_by_sm_rp_mr() may consider</span><br><span style="color: hsl(120, 100%, 40%);">+ * 0x00 as used. This is why we "poison" this transaction</span><br><span style="color: hsl(120, 100%, 40%);">+ * using the highest value. */</span><br><span style="color: hsl(120, 100%, 40%);">+ trans->sms.sm_rp_mr = 0xff;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* According to 8.2.3, MR is in the range 0 through 255 */</span><br><span style="color: hsl(120, 100%, 40%);">+ for (mr = 0x00; mr < 0xff; mr++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (trans_find_by_sm_rp_mr(trans->net, trans->vsub, mr))</span><br><span style="color: hsl(120, 100%, 40%);">+ continue; /* this MR is busy, find another one */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* An unused value has been found, assign it */</span><br><span style="color: hsl(120, 100%, 40%);">+ trans->sms.sm_rp_mr = mr;</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</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%);">+ /* All possible values are busy */</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EBUSY;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static struct gsm_trans *gsm411_alloc_mt_trans(struct gsm_network *net,</span><br><span> struct vlr_subscr *vsub)</span><br><span> {</span><br><span>@@ -1052,9 +1076,11 @@</span><br><span> if (!trans)</span><br><span> return NULL;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (conn) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* Generate unique RP Message Reference */</span><br><span style="color: hsl(0, 100%, 40%);">- trans->sms.sm_rp_mr = conn->next_rp_ref++;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Assign a unique SM-RP Message Reference */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (gsm411_assign_sm_rp_mr(trans) != 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DLSMS, LOGL_ERROR, "Failed to assign SM-RP-MR\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ trans_free(trans);</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span> }</span><br><span> </span><br><span> /* Use SAPI 3 (see GSM 04.11, section 2.3) */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/12628">change 12628</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/12628"/><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-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ife6d954c46b7d8348a4221ab677d0355eb3ee7ac </div>
<div style="display:none"> Gerrit-Change-Number: 12628 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>