[MERGED] osmo-pcu[master]: dl tbf: initialize punct values and verify

[Neels Hofmeyr]

Neels Hofmeyr has submitted this change and it was merged.

Change subject: dl tbf: initialize punct values and verify
......................................................................


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
---
M src/rlc.cpp
M src/tbf_dl.cpp
2 files changed, 38 insertions(+), 4 deletions(-)

Approvals:
  arvind.sirsikar: Looks good to me, but someone else must approve
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/rlc.cpp b/src/rlc.cpp
index d13045e..acd4169 100644
--- a/src/rlc.cpp
+++ b/src/rlc.cpp
@@ -378,6 +378,36 @@
 	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 c04a84e..d871c4d 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -627,10 +627,16 @@
 	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 @@
 	}
 
 	/* 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 */

-- 
To view, visit https://gerrit.osmocom.org/1775
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I004cbbab15e6ffa2749f4b7f1df651517c2ae693
Gerrit-PatchSet: 3
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: arvind.sirsikar <arvind.sirsikar at radisys.com>