From 881064e9b81de4aee7a9cdd52184860260f8723c Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 14 Dec 2016 14:51:40 +0100 Subject: [PATCH] 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 --- openbsc/include/openbsc/system_information.h | 4 ++ openbsc/src/libbsc/arfcn_range_encode.c | 4 ++ openbsc/src/libbsc/rest_octets.c | 21 +++++-- openbsc/src/libbsc/system_information.c | 66 +++++++++++--------- openbsc/tests/gsm0408/gsm0408_test.c | 16 +++++ openbsc/tests/gsm0408/gsm0408_test.ok | 4 ++ 6 files changed, 78 insertions(+), 37 deletions(-) diff --git a/openbsc/include/openbsc/system_information.h b/openbsc/include/openbsc/system_information.h index ebc3afd7e..1b19c8bc5 100644 --- a/openbsc/include/openbsc/system_information.h +++ b/openbsc/include/openbsc/system_information.h @@ -3,6 +3,8 @@ #include +#include + struct gsm_bts; int gsm_generate_si(struct gsm_bts *bts, enum osmo_sysinfo_type type); @@ -10,6 +12,8 @@ unsigned uarfcn_size(const uint16_t *u, const uint16_t *sc, size_t u_len); 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 99188384d..9ca48407e 100644 --- a/openbsc/src/libbsc/arfcn_range_encode.c +++ b/openbsc/src/libbsc/arfcn_range_encode.c @@ -27,6 +27,8 @@ #include +#include + static inline int greatest_power_of_2_lesser_or_equal_to(int index) { int power_of_2 = 1; @@ -109,6 +111,8 @@ int range_enc_arfcns(enum gsm48_range range, /* 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 1a5e43525..fc0282e06 100644 --- a/openbsc/src/libbsc/rest_octets.c +++ b/openbsc/src/libbsc/rest_octets.c @@ -180,10 +180,10 @@ static inline void append_earfcn(struct bitvec *bv, 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 @@ static inline void append_uarfcn(struct bitvec *bv, const uint16_t *u, /* 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 @@ static inline void append_uarfcn(struct bitvec *bv, const uint16_t *u, /* 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 @@ int rest_octets_si2quater(uint8_t *data, const struct osmo_earfcn_si2q *e, 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 3d55d1a73..20c3915e2 100644 --- a/openbsc/src/libbsc/system_information.c +++ b/openbsc/src/libbsc/system_information.c @@ -316,6 +316,38 @@ static inline int enc_freq_lst_var_bitmap(uint8_t *chan_list, 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 @@ static inline int enc_freq_lst_range(uint8_t *chan_list, { 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 @@ static inline int enc_freq_lst_range(uint8_t *chan_list, 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 @@ static int bitvec2freq_list(uint8_t *chan_list, struct bitvec *bv, /* 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 ae8cbdd0f..472c2aeef 100644 --- a/openbsc/tests/gsm0408/gsm0408_test.c +++ b/openbsc/tests/gsm0408/gsm0408_test.c @@ -120,6 +120,21 @@ static inline void _bts_uarfcn_add(struct gsm_bts *bts, uint16_t arfcn, 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 @@ int main(int argc, char **argv) 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 ebe94762e..1118dd940 100644 --- a/openbsc/tests/gsm0408/gsm0408_test.ok +++ b/openbsc/tests/gsm0408/gsm0408_test.ok @@ -62,6 +62,10 @@ testing RP-Reference wrap 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