From 8f04fa975819113f2a75a1803cbc3d06877fd36d Mon Sep 17 00:00:00 2001
From: Oliver Smith <osmith@sysmocom.de>
Date: Mon, 4 May 2020 11:47:13 +0200
Subject: mobile: traffic req check: support EFR

L1CTL handling code should not be involved in such high level checks, so
while at it, move the check into a separate function in gsm48_rr.c and
add a length check. gsm48_rr_tx_voice() is the only caller of
l1ctl_tx_traffic_req().

Related: SYS#4924
Change-Id: Iba84f5d60ff5b1a2db8fb6af5131e185965df7c9
---
 src/host/layer23/src/common/l1ctl.c    |  7 -----
 src/host/layer23/src/mobile/gsm48_rr.c | 51 +++++++++++++++++++++++++++++++---
 2 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/src/host/layer23/src/common/l1ctl.c b/src/host/layer23/src/common/l1ctl.c
index b7d0ecd6..54c7452a 100644
--- a/src/host/layer23/src/common/l1ctl.c
+++ b/src/host/layer23/src/common/l1ctl.c
@@ -909,13 +909,6 @@ int l1ctl_tx_traffic_req(struct osmocom_ms *ms, struct msgb *msg,
 	DEBUGP(DL1C, "TRAFFIC REQ len=%zu (%s)\n", frame_len,
 		osmo_hexdump(frame, frame_len));
 
-	if ((frame[0] >> 4) != 0xd) {
-		LOGP(DL1C, LOGL_ERROR, "Traffic Request has incorrect magic "
-			"(%u != 0xd)\n", frame[0] >> 4);
-		msgb_free(msg);
-		return -EINVAL;
-	}
-
 //	printf("TX %s\n", osmo_hexdump(frame, frame_len));
 
 	/* prepend uplink info header */
diff --git a/src/host/layer23/src/mobile/gsm48_rr.c b/src/host/layer23/src/mobile/gsm48_rr.c
index b3da258b..fdc99168 100644
--- a/src/host/layer23/src/mobile/gsm48_rr.c
+++ b/src/host/layer23/src/mobile/gsm48_rr.c
@@ -71,6 +71,7 @@
 #include <osmocom/gsm/rsl.h>
 #include <osmocom/gsm/gsm48.h>
 #include <osmocom/core/bitvec.h>
+#include <osmocom/codec/codec.h>
 
 #include <osmocom/bb/common/osmocom_data.h>
 #include <osmocom/bb/common/l1l2_interface.h>
@@ -5624,25 +5625,67 @@ static int gsm48_rr_rand_acc_cnf_dedicated(struct osmocom_ms *ms, struct msgb *m
 
 #endif
 
+#define LOG_FRAME_VERIFY(mode, level, fmt, args...) \
+	LOGP(DRR, level, "Voice frame, mode=%s: " fmt, get_value_string(gsm48_chan_mode_names, mode), ## args)
+
+int voice_frame_verify(enum gsm48_chan_mode mode, uint8_t *frame, size_t frame_len)
+{
+	switch (mode) {
+	case GSM48_CMODE_SPEECH_V1:
+		/* FIXME: this is FR only, check for TCH/F (FR) and TCH/H (HR) */
+		/* RFC 3551, section 4.5.8 GSM */
+		if (frame_len != GSM_FR_BYTES) {
+			LOG_FRAME_VERIFY(mode, LOGL_ERROR, "incorrect length (%zu != %u)\n", frame_len, GSM_FR_BYTES);
+			return -2;
+		}
+		if ((frame[0] >> 4) != 0xd) {
+			LOG_FRAME_VERIFY(mode, LOGL_ERROR, "incorrect magic (%u != 0xd)\n", frame[0] >> 4);
+			return -3;
+		}
+		break;
+	case GSM48_CMODE_SPEECH_EFR:
+		/* RFC 3551, section 4.5.9 GSM-EFR */
+		if (frame_len != GSM_EFR_BYTES) {
+			LOG_FRAME_VERIFY(mode, LOGL_ERROR, "incorrect length (%zu != %u)\n", frame_len, GSM_EFR_BYTES);
+			return -4;
+		}
+		if ((frame[0] >> 4) != 0xc) {
+			LOG_FRAME_VERIFY(mode, LOGL_ERROR, "incorrect magic (%u != 0xc)\n", frame[0] >> 4);
+			return -5;
+		}
+		break;
+	default:
+		LOG_FRAME_VERIFY(mode, LOGL_ERROR, "not implemented\n");
+		return -1;
+	}
+	return 0;
+}
+
 int gsm48_rr_tx_voice(struct osmocom_ms *ms, struct msgb *msg)
 {
 	struct gsm48_rrlayer *rr = &ms->rrlayer;
 	uint8_t ch_type, ch_subch, ch_ts;
+	struct l1ctl_traffic_req *tr;
 
 	if (!rr->dm_est) {
 		LOGP(DRR, LOGL_INFO, "Current channel is not active\n");
-		msgb_free(msg);
-		return -ENOTSUP;
+		goto error;
 	}
 
 	rsl_dec_chan_nr(rr->cd_now.chan_nr, &ch_type, &ch_subch, &ch_ts);
 	if (ch_type != RSL_CHAN_Bm_ACCHs) {
 		LOGP(DRR, LOGL_INFO, "Current channel is not (yet) TCH/F\n");
-		msgb_free(msg);
-		return -ENOTSUP;
+		goto error;
 	}
 
+	tr = (struct l1ctl_traffic_req *)msg->l2h;
+	if (voice_frame_verify(rr->cd_now.mode, tr->data, msgb_l2len(msg)) < 0)
+		goto error;
+
 	return l1ctl_tx_traffic_req(ms, msg, rr->cd_now.chan_nr, 0);
+error:
+	msgb_free(msg);
+	return -ENOTSUP;
 }
 
 int gsm48_rr_audio_mode(struct osmocom_ms *ms, uint8_t mode)
-- 
cgit v1.2.3