<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmocom-bb/+/18021">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">mobile: traffic req check: support EFR<br><br>L1CTL handling code should not be involved in such high level checks, so<br>while at it, move the check into a separate function in gsm48_rr.c and<br>add a length check. gsm48_rr_tx_voice() is the only caller of<br>l1ctl_tx_traffic_req().<br><br>Related: SYS#4924<br>Change-Id: Iba84f5d60ff5b1a2db8fb6af5131e185965df7c9<br>---<br>M src/host/layer23/src/common/l1ctl.c<br>M src/host/layer23/src/mobile/gsm48_rr.c<br>2 files changed, 47 insertions(+), 11 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/host/layer23/src/common/l1ctl.c b/src/host/layer23/src/common/l1ctl.c</span><br><span>index b7d0ecd..54c7452 100644</span><br><span>--- a/src/host/layer23/src/common/l1ctl.c</span><br><span>+++ b/src/host/layer23/src/common/l1ctl.c</span><br><span>@@ -909,13 +909,6 @@</span><br><span>     DEBUGP(DL1C, "TRAFFIC REQ len=%zu (%s)\n", frame_len,</span><br><span>              osmo_hexdump(frame, frame_len));</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    if ((frame[0] >> 4) != 0xd) {</span><br><span style="color: hsl(0, 100%, 40%);">-             LOGP(DL1C, LOGL_ERROR, "Traffic Request has incorrect magic "</span><br><span style="color: hsl(0, 100%, 40%);">-                 "(%u != 0xd)\n", frame[0] >> 4);</span><br><span style="color: hsl(0, 100%, 40%);">-                msgb_free(msg);</span><br><span style="color: hsl(0, 100%, 40%);">-         return -EINVAL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> // printf("TX %s\n", osmo_hexdump(frame, frame_len));</span><br><span> </span><br><span>     /* prepend uplink info header */</span><br><span>diff --git a/src/host/layer23/src/mobile/gsm48_rr.c b/src/host/layer23/src/mobile/gsm48_rr.c</span><br><span>index b3da258..fdc9916 100644</span><br><span>--- a/src/host/layer23/src/mobile/gsm48_rr.c</span><br><span>+++ b/src/host/layer23/src/mobile/gsm48_rr.c</span><br><span>@@ -71,6 +71,7 @@</span><br><span> #include <osmocom/gsm/rsl.h></span><br><span> #include <osmocom/gsm/gsm48.h></span><br><span> #include <osmocom/core/bitvec.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/codec/codec.h></span><br><span> </span><br><span> #include <osmocom/bb/common/osmocom_data.h></span><br><span> #include <osmocom/bb/common/l1l2_interface.h></span><br><span>@@ -5624,25 +5625,67 @@</span><br><span> </span><br><span> #endif</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define LOG_FRAME_VERIFY(mode, level, fmt, args...) \</span><br><span style="color: hsl(120, 100%, 40%);">+  LOGP(DRR, level, "Voice frame, mode=%s: " fmt, get_value_string(gsm48_chan_mode_names, mode), ## args)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+int voice_frame_verify(enum gsm48_chan_mode mode, uint8_t *frame, size_t frame_len)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+    switch (mode) {</span><br><span style="color: hsl(120, 100%, 40%);">+       case GSM48_CMODE_SPEECH_V1:</span><br><span style="color: hsl(120, 100%, 40%);">+           /* FIXME: this is FR only, check for TCH/F (FR) and TCH/H (HR) */</span><br><span style="color: hsl(120, 100%, 40%);">+             /* RFC 3551, section 4.5.8 GSM */</span><br><span style="color: hsl(120, 100%, 40%);">+             if (frame_len != GSM_FR_BYTES) {</span><br><span style="color: hsl(120, 100%, 40%);">+                      LOG_FRAME_VERIFY(mode, LOGL_ERROR, "incorrect length (%zu != %u)\n", frame_len, GSM_FR_BYTES);</span><br><span style="color: hsl(120, 100%, 40%);">+                      return -2;</span><br><span style="color: hsl(120, 100%, 40%);">+            }</span><br><span style="color: hsl(120, 100%, 40%);">+             if ((frame[0] >> 4) != 0xd) {</span><br><span style="color: hsl(120, 100%, 40%);">+                   LOG_FRAME_VERIFY(mode, LOGL_ERROR, "incorrect magic (%u != 0xd)\n", frame[0] >> 4);</span><br><span style="color: hsl(120, 100%, 40%);">+                   return -3;</span><br><span style="color: hsl(120, 100%, 40%);">+            }</span><br><span style="color: hsl(120, 100%, 40%);">+             break;</span><br><span style="color: hsl(120, 100%, 40%);">+        case GSM48_CMODE_SPEECH_EFR:</span><br><span style="color: hsl(120, 100%, 40%);">+          /* RFC 3551, section 4.5.9 GSM-EFR */</span><br><span style="color: hsl(120, 100%, 40%);">+         if (frame_len != GSM_EFR_BYTES) {</span><br><span style="color: hsl(120, 100%, 40%);">+                     LOG_FRAME_VERIFY(mode, LOGL_ERROR, "incorrect length (%zu != %u)\n", frame_len, GSM_EFR_BYTES);</span><br><span style="color: hsl(120, 100%, 40%);">+                     return -4;</span><br><span style="color: hsl(120, 100%, 40%);">+            }</span><br><span style="color: hsl(120, 100%, 40%);">+             if ((frame[0] >> 4) != 0xc) {</span><br><span style="color: hsl(120, 100%, 40%);">+                   LOG_FRAME_VERIFY(mode, LOGL_ERROR, "incorrect magic (%u != 0xc)\n", frame[0] >> 4);</span><br><span style="color: hsl(120, 100%, 40%);">+                   return -5;</span><br><span style="color: hsl(120, 100%, 40%);">+            }</span><br><span style="color: hsl(120, 100%, 40%);">+             break;</span><br><span style="color: hsl(120, 100%, 40%);">+        default:</span><br><span style="color: hsl(120, 100%, 40%);">+              LOG_FRAME_VERIFY(mode, LOGL_ERROR, "not implemented\n");</span><br><span style="color: hsl(120, 100%, 40%);">+            return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+    }</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> int gsm48_rr_tx_voice(struct osmocom_ms *ms, struct msgb *msg)</span><br><span> {</span><br><span>        struct gsm48_rrlayer *rr = &ms->rrlayer;</span><br><span>      uint8_t ch_type, ch_subch, ch_ts;</span><br><span style="color: hsl(120, 100%, 40%);">+     struct l1ctl_traffic_req *tr;</span><br><span> </span><br><span>    if (!rr->dm_est) {</span><br><span>                LOGP(DRR, LOGL_INFO, "Current channel is not active\n");</span><br><span style="color: hsl(0, 100%, 40%);">-              msgb_free(msg);</span><br><span style="color: hsl(0, 100%, 40%);">-         return -ENOTSUP;</span><br><span style="color: hsl(120, 100%, 40%);">+              goto error;</span><br><span>  }</span><br><span> </span><br><span>        rsl_dec_chan_nr(rr->cd_now.chan_nr, &ch_type, &ch_subch, &ch_ts);</span><br><span>     if (ch_type != RSL_CHAN_Bm_ACCHs) {</span><br><span>          LOGP(DRR, LOGL_INFO, "Current channel is not (yet) TCH/F\n");</span><br><span style="color: hsl(0, 100%, 40%);">-         msgb_free(msg);</span><br><span style="color: hsl(0, 100%, 40%);">-         return -ENOTSUP;</span><br><span style="color: hsl(120, 100%, 40%);">+              goto error;</span><br><span>  }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ tr = (struct l1ctl_traffic_req *)msg->l2h;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (voice_frame_verify(rr->cd_now.mode, tr->data, msgb_l2len(msg)) < 0)</span><br><span style="color: hsl(120, 100%, 40%);">+              goto error;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>        return l1ctl_tx_traffic_req(ms, msg, rr->cd_now.chan_nr, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+error:</span><br><span style="color: hsl(120, 100%, 40%);">+       msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+       return -ENOTSUP;</span><br><span> }</span><br><span> </span><br><span> int gsm48_rr_audio_mode(struct osmocom_ms *ms, uint8_t mode)</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmocom-bb/+/18021">change 18021</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/osmocom-bb/+/18021"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmocom-bb </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Iba84f5d60ff5b1a2db8fb6af5131e185965df7c9 </div>
<div style="display:none"> Gerrit-Change-Number: 18021 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>