[MERGED] openbsc[master]: Prevent segfault in range encoding

Harald Welte gerrit-no-reply at lists.osmocom.org
Mon Jan 23 12:32:00 UTC 2017


Harald Welte has submitted this change and it was merged.

Change subject: Prevent segfault in range encoding
......................................................................


Prevent segfault in range encoding

* Explicitly check when ARFCN array split is impossible and return
  gracefully instead of using negative index.
* Separate range encoding into generic function and use it for all
  SI-related things.
* Propagate the error into that function and to its callers.
* Add separate test-case for the segfault previously triggered by this bug.

Change-Id: I3e049ab2d7c1c4d6c791b148f37e10636a8e43e0
Related: RT#7379
---
M openbsc/include/openbsc/system_information.h
M openbsc/src/libbsc/arfcn_range_encode.c
M openbsc/src/libbsc/rest_octets.c
M openbsc/src/libbsc/system_information.c
M openbsc/tests/gsm0408/gsm0408_test.c
M openbsc/tests/gsm0408/gsm0408_test.ok
6 files changed, 78 insertions(+), 37 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/openbsc/include/openbsc/system_information.h b/openbsc/include/openbsc/system_information.h
index ebc3afd..1b19c8b 100644
--- a/openbsc/include/openbsc/system_information.h
+++ b/openbsc/include/openbsc/system_information.h
@@ -3,6 +3,8 @@
 
 #include <osmocom/gsm/sysinfo.h>
 
+#include <openbsc/arfcn_range_encode.h>
+
 struct gsm_bts;
 
 int gsm_generate_si(struct gsm_bts *bts, enum osmo_sysinfo_type type);
@@ -10,6 +12,8 @@
 unsigned earfcn_size(const struct osmo_earfcn_si2q *e);
 unsigned range1024_p(unsigned n);
 unsigned range512_q(unsigned m);
+int range_encode(enum gsm48_range r, int *arfcns, int arfcns_used, int *w,
+		 int f0, uint8_t *chan_list);
 bool si2q_size_check(const struct gsm_bts *bts);
 int bts_uarfcn_del(struct gsm_bts *bts, uint16_t arfcn, uint16_t scramble);
 int bts_uarfcn_add(struct gsm_bts *bts, uint16_t arfcn, uint16_t scramble,
diff --git a/openbsc/src/libbsc/arfcn_range_encode.c b/openbsc/src/libbsc/arfcn_range_encode.c
index 9918838..9ca4840 100644
--- a/openbsc/src/libbsc/arfcn_range_encode.c
+++ b/openbsc/src/libbsc/arfcn_range_encode.c
@@ -27,6 +27,8 @@
 
 #include <osmocom/core/utils.h>
 
+#include <errno.h>
+
 static inline int greatest_power_of_2_lesser_or_equal_to(int index)
 {
 	int power_of_2 = 1;
@@ -109,6 +111,8 @@
 
 	/* Now do the processing */
 	split_at = range_enc_find_index(range, arfcns, size);
+	if (split_at < 0)
+		return -EINVAL;
 
 	/* we now know where to split */
 	out[index] = 1 + arfcns[split_at];
diff --git a/openbsc/src/libbsc/rest_octets.c b/openbsc/src/libbsc/rest_octets.c
index 1a5e435..fc0282e 100644
--- a/openbsc/src/libbsc/rest_octets.c
+++ b/openbsc/src/libbsc/rest_octets.c
@@ -180,10 +180,10 @@
 	bitvec_set_bit(bv, L);
 }
 
-static inline void append_uarfcn(struct bitvec *bv, const uint16_t *u,
+static inline int append_uarfcn(struct bitvec *bv, const uint16_t *u,
 				 const uint16_t *sc, size_t length)
 {
-	int f0_inc, i, arfcns_used, w[RANGE_ENC_MAX_ARFCNS], a[length];
+	int f0_inc, i, w[RANGE_ENC_MAX_ARFCNS] = { 0 }, a[length];
 	uint8_t chan_list[16] = {0};
 
 	/* 3G Neighbour Cell Description */
@@ -211,9 +211,9 @@
 	/* Note: we do not support multiple UARFCN values ATM: */
 	bitvec_set_uint(bv, u[0], 14);
 
-	arfcns_used = range_enc_filter_arfcns(a, length, 0, &f0_inc);
-	range_enc_arfcns(ARFCN_RANGE_1024, a, arfcns_used, w, 0);
-	range_enc_range1024(chan_list, 0, f0_inc, w);
+	f0_inc = range_encode(ARFCN_RANGE_1024, a, length, w, 0, chan_list);
+	if (f0_inc < 0)
+		return f0_inc;
 
 	/* FDD_Indic0: parameter value '0000000000' is not a member of the set */
 	bitvec_set_bit(bv, f0_inc);
@@ -229,12 +229,15 @@
 
 	/* UTRAN TDD Description */
 	bitvec_set_bit(bv, 0);
+
+	return 0;
 }
 
 /* generate SI2quater rest octets: 3GPP TS 44.018 § 10.5.2.33b */
 int rest_octets_si2quater(uint8_t *data, const struct osmo_earfcn_si2q *e,
 			  const uint16_t *u, const uint16_t *sc, size_t u_len)
 {
+	int rc;
 	unsigned sz;
 	struct bitvec bv;
 	bv.data = data;
@@ -279,7 +282,13 @@
 			     SI2Q_MAX_LEN);
 			return -ENOMEM;
 		}
-		append_uarfcn(&bv, u, sc, u_len);
+		rc = append_uarfcn(&bv, u, sc, u_len);
+		if (rc < 0) {
+			LOGP(DRR, LOGL_ERROR, "SI2quater: failed to append %zu "
+			     "UARFCNs due to range encoding failure: %s\n",
+			     u_len, strerror(-rc));
+			return rc;
+		}
 	} else { /* No 3G Neighbour Cell Description */
 		bitvec_set_bit(&bv, 0);
 	}
diff --git a/openbsc/src/libbsc/system_information.c b/openbsc/src/libbsc/system_information.c
index 3d55d1a..20c3915 100644
--- a/openbsc/src/libbsc/system_information.c
+++ b/openbsc/src/libbsc/system_information.c
@@ -316,6 +316,38 @@
 	return 0;
 }
 
+int range_encode(enum gsm48_range r, int *arfcns, int arfcns_used, int *w,
+		 int f0, uint8_t *chan_list)
+{
+	/*
+	 * Manipulate the ARFCN list according to the rules in J4 depending
+	 * on the selected range.
+	 */
+	int rc, f0_included;
+
+	range_enc_filter_arfcns(arfcns, arfcns_used, f0, &f0_included);
+
+	rc = range_enc_arfcns(r, arfcns, arfcns_used, w, 0);
+	if (rc < 0)
+		return rc;
+
+	/* Select the range and the amount of bits needed */
+	switch (r) {
+	case ARFCN_RANGE_128:
+		return range_enc_range128(chan_list, f0, w);
+	case ARFCN_RANGE_256:
+		return range_enc_range256(chan_list, f0, w);
+	case ARFCN_RANGE_512:
+		return range_enc_range512(chan_list, f0, w);
+	case ARFCN_RANGE_1024:
+		return range_enc_range1024(chan_list, f0, f0_included, w);
+	default:
+		return -ERANGE;
+	};
+
+	return f0_included;
+}
+
 /* generate a frequency list with the range 512 format */
 static inline int enc_freq_lst_range(uint8_t *chan_list,
 				struct bitvec *bv, const struct gsm_bts *bts,
@@ -323,9 +355,8 @@
 {
 	int arfcns[RANGE_ENC_MAX_ARFCNS];
 	int w[RANGE_ENC_MAX_ARFCNS];
-	int f0_included = 0;
 	int arfcns_used = 0;
-	int i, rc, range, f0;
+	int i, range, f0;
 
 	/*
 	 * Select ARFCNs according to the rules in bitvec2freq_list
@@ -346,35 +377,8 @@
 	if (range == ARFCN_RANGE_INVALID)
 		return -2;
 
-	/*
-	 * Manipulate the ARFCN list according to the rules in J4 depending
-	 * on the selected range.
-	 */
-	arfcns_used = range_enc_filter_arfcns(arfcns, arfcns_used,
-				f0, &f0_included);
-
 	memset(w, 0, sizeof(w));
-	rc = range_enc_arfcns(range, arfcns, arfcns_used, w, 0);
-	if (rc != 0)
-		return -3;
-
-	/* Select the range and the amount of bits needed */
-	switch (range) {
-	case ARFCN_RANGE_128:
-		return range_enc_range128(chan_list, f0, w);
-		break;
-	case ARFCN_RANGE_256:
-		return range_enc_range256(chan_list, f0, w);
-		break;
-	case ARFCN_RANGE_512:
-		return range_enc_range512(chan_list, f0, w);
-		break;
-	case ARFCN_RANGE_1024:
-		return range_enc_range1024(chan_list, f0, f0_included, w);
-		break;
-	default:
-		return -4;
-	};
+	return range_encode(range, arfcns, arfcns_used, w, f0, chan_list);
 }
 
 /* generate a cell channel list as per Section 10.5.2.1b of 04.08 */
@@ -447,7 +451,7 @@
 
 	/* Attempt to do the range encoding */
 	rc = enc_freq_lst_range(chan_list, bv, bts, bis, ter, pgsm);
-	if (rc == 0)
+	if (rc >= 0)
 		return 0;
 
 	LOGP(DRR, LOGL_ERROR, "min_arfcn=%u, max_arfcn=%u, arfcns=%d "
diff --git a/openbsc/tests/gsm0408/gsm0408_test.c b/openbsc/tests/gsm0408/gsm0408_test.c
index ae8cbdd..472c2ae 100644
--- a/openbsc/tests/gsm0408/gsm0408_test.c
+++ b/openbsc/tests/gsm0408/gsm0408_test.c
@@ -120,6 +120,21 @@
 		gen(bts);
 }
 
+static inline void test_si2q_segfault(void)
+{
+	struct gsm_bts *bts;
+	struct gsm_network *network = bsc_network_init(tall_bsc_ctx, 1, 1, NULL);
+	printf("Test SI2quater UARFCN (same scrambling code and diversity):\n");
+
+	if (!network)
+		exit(1);
+	bts = gsm_bts_alloc(network);
+
+	_bts_uarfcn_add(bts, 10564, 319, 0);
+	_bts_uarfcn_add(bts, 10612, 319, 0);
+	gen(bts);
+}
+
 static inline void test_si2q_u(void)
 {
 	struct gsm_bts *bts;
@@ -590,6 +605,7 @@
 	test_range_encoding();
 	test_gsm411_rp_ref_wrap();
 
+	test_si2q_segfault();
 	test_si2q_e();
 	test_si2q_u();
 	printf("Done.\n");
diff --git a/openbsc/tests/gsm0408/gsm0408_test.ok b/openbsc/tests/gsm0408/gsm0408_test.ok
index ebe9476..1118dd9 100644
--- a/openbsc/tests/gsm0408/gsm0408_test.ok
+++ b/openbsc/tests/gsm0408/gsm0408_test.ok
@@ -62,6 +62,10 @@
 Allocated reference: 255
 Allocated reference: 0
 Allocated reference: 1
+Test SI2quater UARFCN (same scrambling code and diversity):
+generated valid SI2quater: [23] 59 06 07 c0 00 25 52 88 0a 7e 10 99 64 00 0b 2b 2b 2b 2b 2b 2b 2b 2b 
+failed to generate SI2quater: Invalid argument
+failed to generate SI2quater: Invalid argument
 Testing SYSINFO_TYPE_2quater EARFCN generation:
 generated invalid SI2quater: [23] 59 06 07 c0 00 04 86 59 0a 03 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
 added EARFCN 1917 - generated valid SI2quater: [23] 59 06 07 c0 00 04 86 59 83 be c8 50 0b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3e049ab2d7c1c4d6c791b148f37e10636a8e43e0
Gerrit-PatchSet: 7
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>


More information about the gerrit-log mailing list