[PATCH] osmo-pcu[master]: Sanitizer fix for invalid value of egprs_puncturing_values

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

arvind.sirsikar gerrit-no-reply at lists.osmocom.org
Thu Jan 12 12:51:38 UTC 2017


Hello Neels Hofmeyr, Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/1411

to look at the new patch set (#4).

Sanitizer fix for invalid value of egprs_puncturing_values

The puncturing values are set to EGPRS_PS_INVALID initially for
both the BSNs and CPS calculation is done for only EGPRS case.
During CPS calculation each puncturing values are validated
against EGPRS_PS_INVALID to ensure the correctness of the variable.

Change-Id: Ice54edc7e4a936eb2f2dd8a243673a30dceef542
---
M src/rlc.cpp
M src/tbf_dl.cpp
2 files changed, 46 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/11/1411/4

diff --git a/src/rlc.cpp b/src/rlc.cpp
index 2bffccb..0dfec8f 100644
--- a/src/rlc.cpp
+++ b/src/rlc.cpp
@@ -351,26 +351,49 @@
 	enum egprs_puncturing_values punct2, int with_padding)
 {
 	switch (GprsCodingScheme::Scheme(cs)) {
-	case GprsCodingScheme::MCS1: return 0b1011 +
-		punct % EGPRS_MAX_PS_NUM_2;
-	case GprsCodingScheme::MCS2: return 0b1001 +
-		punct % EGPRS_MAX_PS_NUM_2;
-	case GprsCodingScheme::MCS3: return (with_padding ? 0b0110 : 0b0011) +
+	case GprsCodingScheme::MCS1:
+		OSMO_ASSERT(punct != EGPRS_PS_INVALID);
+
+		return 0b1011 +	punct % EGPRS_MAX_PS_NUM_2;
+	case GprsCodingScheme::MCS2:
+		OSMO_ASSERT(punct != EGPRS_PS_INVALID);
+
+		return 0b1001 +	punct % EGPRS_MAX_PS_NUM_2;
+	case GprsCodingScheme::MCS3:
+		OSMO_ASSERT(punct != EGPRS_PS_INVALID);
+
+		return (with_padding ? 0b0110 : 0b0011) +
 		punct % EGPRS_MAX_PS_NUM_3;
-	case GprsCodingScheme::MCS4: return 0b0000 +
-		punct % EGPRS_MAX_PS_NUM_3;
-	case GprsCodingScheme::MCS5: return  0b100 +
+	case GprsCodingScheme::MCS4:
+		OSMO_ASSERT(punct != EGPRS_PS_INVALID);
+
+		return 0b0000 + punct % EGPRS_MAX_PS_NUM_3;
+	case GprsCodingScheme::MCS5:
+		OSMO_ASSERT(punct != EGPRS_PS_INVALID);
+
+		return  0b100 + punct % EGPRS_MAX_PS_NUM_2;
+	case GprsCodingScheme::MCS6:
+		OSMO_ASSERT(punct != EGPRS_PS_INVALID);
+
+		return (with_padding ? 0b010 : 0b000) +
 		punct % EGPRS_MAX_PS_NUM_2;
-	case GprsCodingScheme::MCS6: return (with_padding ? 0b010 : 0b000) +
-		punct % EGPRS_MAX_PS_NUM_2;
-	case GprsCodingScheme::MCS7: return 0b10100 +
-		3 * (punct % EGPRS_MAX_PS_NUM_3) +
+	case GprsCodingScheme::MCS7:
+		OSMO_ASSERT(punct != EGPRS_PS_INVALID);
+		OSMO_ASSERT(punct2 != EGPRS_PS_INVALID);
+
+		return 0b10100 + 3 * (punct % EGPRS_MAX_PS_NUM_3) +
 		punct2 % EGPRS_MAX_PS_NUM_3;
-	case GprsCodingScheme::MCS8: return 0b01011 +
-		3 * (punct % EGPRS_MAX_PS_NUM_3) +
+	case GprsCodingScheme::MCS8:
+		OSMO_ASSERT(punct != EGPRS_PS_INVALID);
+		OSMO_ASSERT(punct2 != EGPRS_PS_INVALID);
+
+		return 0b01011 + 3 * (punct % EGPRS_MAX_PS_NUM_3) +
 		punct2 % EGPRS_MAX_PS_NUM_3;
-	case GprsCodingScheme::MCS9: return 0b00000 +
-		4 * (punct % EGPRS_MAX_PS_NUM_3) +
+	case GprsCodingScheme::MCS9:
+		OSMO_ASSERT(punct != EGPRS_PS_INVALID);
+		OSMO_ASSERT(punct2 != EGPRS_PS_INVALID);
+
+		return 0b00000 + 4 * (punct % EGPRS_MAX_PS_NUM_3) +
 		punct2 % EGPRS_MAX_PS_NUM_3;
 	default: ;
 	}
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index 78f06e9..e2b2e68 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -631,6 +631,10 @@
 	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);
+
+	/* Initialise the puncturing schemes for both BSN as to Invalid */
+	memset(punct, EGPRS_PS_INVALID, sizeof(punct));
+
 	/*
 	 * 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
@@ -762,8 +766,9 @@
 			msg_data, block_data);
 	}
 
-	OSMO_ASSERT(ARRAY_SIZE(punct) >= 2);
-	rlc.cps = gprs_rlc_mcs_cps(cs, punct[0], punct[1], need_padding);
+	/* Calculate CPS only for EGPRS case */
+	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/1411
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ice54edc7e4a936eb2f2dd8a243673a30dceef542
Gerrit-PatchSet: 4
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: arvind.sirsikar <arvind.sirsikar at radisys.com>
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>



More information about the gerrit-log mailing list