From b609190369bdf96eefedacc012503d7b55823302 Mon Sep 17 00:00:00 2001
From: Neels Hofmeyr <nhofmeyr@sysmocom.de>
Date: Wed, 8 Feb 2017 16:49:20 +0100
Subject: [PATCH] dl tbf: initialize punct values and verify

Solves a sanitizer issue where punct2 is unset when passed to
gprs_rlc_mcs_cps() and thus takes a value not defined in the enum.

Change-Id: I004cbbab15e6ffa2749f4b7f1df651517c2ae693
---
 src/rlc.cpp    | 30 ++++++++++++++++++++++++++++++
 src/tbf_dl.cpp | 12 ++++++++----
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/src/rlc.cpp b/src/rlc.cpp
index d13045e8..acd41693 100644
--- a/src/rlc.cpp
+++ b/src/rlc.cpp
@@ -378,6 +378,36 @@ unsigned int gprs_rlc_mcs_cps(GprsCodingScheme cs,
 	enum egprs_puncturing_values punct,
 	enum egprs_puncturing_values punct2, int with_padding)
 {
+	/* validate that punct and punct2 are as expected */
+	switch (GprsCodingScheme::Scheme(cs)) {
+	case GprsCodingScheme::MCS9:
+	case GprsCodingScheme::MCS8:
+	case GprsCodingScheme::MCS7:
+		if (punct2 == EGPRS_PS_INVALID) {
+			LOGP(DRLCMACDL, LOGL_ERROR,
+			     "Invalid punct2 value for coding scheme %d: %d\n",
+			     GprsCodingScheme::Scheme(cs), punct2);
+			return -1;
+		}
+		/* fall through */
+	case GprsCodingScheme::MCS6:
+	case GprsCodingScheme::MCS5:
+	case GprsCodingScheme::MCS4:
+	case GprsCodingScheme::MCS3:
+	case GprsCodingScheme::MCS2:
+	case GprsCodingScheme::MCS1:
+		if (punct == EGPRS_PS_INVALID) {
+			LOGP(DRLCMACDL, LOGL_ERROR,
+			     "Invalid punct value for coding scheme %d: %d\n",
+			     GprsCodingScheme::Scheme(cs), punct);
+			return -1;
+		}
+		break;
+	default:
+		return -1;
+	}
+
+	/* See 3GPP TS 44.060 10.4.8a.3.1, 10.4.8a.2.1, 10.4.8a.1.1 */
 	switch (GprsCodingScheme::Scheme(cs)) {
 	case GprsCodingScheme::MCS1: return 0b1011 +
 		punct % EGPRS_MAX_PS_NUM_2;
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index c04a84e2..d871c4d7 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -627,10 +627,16 @@ struct msgb *gprs_rlcmac_dl_tbf::create_dl_acked_block(
 	GprsCodingScheme cs;
 	int bsns[ARRAY_SIZE(rlc.block_info)];
 	unsigned num_bsns;
-	enum egprs_puncturing_values punct[ARRAY_SIZE(rlc.block_info)];
 	bool need_padding = false;
 	enum egprs_rlcmac_dl_spb spb = EGPRS_RLCMAC_DL_NO_RETX;
 	unsigned int spb_status = get_egprs_dl_spb_status(index);
+
+	enum egprs_puncturing_values punct[2] = {
+		EGPRS_PS_INVALID, EGPRS_PS_INVALID
+	};
+	osmo_static_assert(ARRAY_SIZE(rlc.block_info) == 2,
+			   rlc_block_info_size_is_two);
+
 	/*
 	 * TODO: This is an experimental work-around to put 2 BSN into
 	 * MSC-7 to MCS-9 encoded messages. It just sends the same BSN
@@ -763,10 +769,8 @@ struct msgb *gprs_rlcmac_dl_tbf::create_dl_acked_block(
 	}
 
 	/* Calculate CPS only for EGPRS case */
-	if (cs.isEgprs()) {
-		OSMO_ASSERT(ARRAY_SIZE(punct) >= 2);
+	if (cs.isEgprs())
 		rlc.cps = gprs_rlc_mcs_cps(cs, punct[0], punct[1], need_padding);
-	}
 
 	/* If the TBF has just started, relate frames_since_last_poll to the
 	 * current fn */