laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/18680 )
Change subject: rsl: refactor handling of RSL_IE_MR_CONFIG ...................................................................... rsl: refactor handling of RSL_IE_MR_CONFIG - get rid of gsm_lchan::mr_bts_lv, it's never used anyway, - check IE length in amr_parse_mr_conf() before parsing, - check return code of amr_parse_mr_conf(). Change-Id: Ibfd5845ea429945b352dd14421e86562998d65ca --- M include/osmo-bts/gsm_data_shared.h M src/common/amr.c M src/common/rsl.c 3 files changed, 20 insertions(+), 21 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved Jenkins Builder: Verified diff --git a/include/osmo-bts/gsm_data_shared.h b/include/osmo-bts/gsm_data_shared.h index f010dbc..dc7d39f 100644 --- a/include/osmo-bts/gsm_data_shared.h +++ b/include/osmo-bts/gsm_data_shared.h @@ -157,9 +157,6 @@ uint8_t key[MAX_A5_KEY_LEN]; } encr; - /* AMR bits */ - uint8_t mr_bts_lv[7]; - struct { uint32_t bound_ip; uint32_t connect_ip; diff --git a/src/common/amr.c b/src/common/amr.c index 05d1aaa..837757f 100644 --- a/src/common/amr.c +++ b/src/common/amr.c @@ -78,13 +78,16 @@ int amr_parse_mr_conf(struct amr_multirate_conf *amr_mrc, const uint8_t *mr_conf, unsigned int len) { - uint8_t mr_version = mr_conf[0] >> 5; uint8_t num_codecs = 0; int i, j = 0; - if (mr_version != 1) { - LOGP(DRSL, LOGL_ERROR, "AMR Multirate Version %u unknown\n", - mr_version); + if (len < 2) { + LOGP(DRSL, LOGL_ERROR, "AMR Multirate IE is too short (%u)\n", len); + goto ret_einval; + } + + if ((mr_conf[0] >> 5) != 1) { + LOGP(DRSL, LOGL_ERROR, "AMR Multirate Version %u unknown\n", (mr_conf[0] >> 5)); goto ret_einval; } diff --git a/src/common/rsl.c b/src/common/rsl.c index 41dd243..f057a89 100644 --- a/src/common/rsl.c +++ b/src/common/rsl.c @@ -1209,17 +1209,16 @@ } /* 9.3.52 MultiRate Configuration */ if (TLVP_PRESENT(&tp, RSL_IE_MR_CONFIG)) { - if (TLVP_LEN(&tp, RSL_IE_MR_CONFIG) > sizeof(lchan->mr_bts_lv) - 1) { + rc = amr_parse_mr_conf(&lchan->tch.amr_mr, + TLVP_VAL(&tp, RSL_IE_MR_CONFIG), + TLVP_LEN(&tp, RSL_IE_MR_CONFIG)); + if (rc < 0) { LOGPLCHAN(lchan, DRSL, LOGL_ERROR, "Error parsing MultiRate conf IE\n"); rsl_tx_error_report(msg->trx, RSL_ERR_IE_CONTENT, &dch->chan_nr, NULL, msg); return rsl_tx_chan_act_acknack(lchan, RSL_ERR_IE_CONTENT); } - memcpy(lchan->mr_bts_lv, TLVP_VAL(&tp, RSL_IE_MR_CONFIG) - 1, - TLVP_LEN(&tp, RSL_IE_MR_CONFIG) + 1); - amr_parse_mr_conf(&lchan->tch.amr_mr, TLVP_VAL(&tp, RSL_IE_MR_CONFIG), - TLVP_LEN(&tp, RSL_IE_MR_CONFIG)); - amr_log_mr_conf(DRTP, LOGL_DEBUG, gsm_lchan_name(lchan), - &lchan->tch.amr_mr); + + amr_log_mr_conf(DRTP, LOGL_DEBUG, gsm_lchan_name(lchan), &lchan->tch.amr_mr); lchan->tch.last_cmr = AMR_CMR_NONE; } /* 9.3.53 MultiRate Control */ @@ -1556,6 +1555,7 @@ struct gsm_lchan *lchan = msg->lchan; struct rsl_ie_chan_mode *cm; struct tlv_parsed tp; + int rc; rsl_tlv_parse(&tp, msgb_l3(msg), msgb_l3len(msg)); @@ -1588,17 +1588,16 @@ /* 9.3.52 MultiRate Configuration */ if (TLVP_PRESENT(&tp, RSL_IE_MR_CONFIG)) { - if (TLVP_LEN(&tp, RSL_IE_MR_CONFIG) > sizeof(lchan->mr_bts_lv) - 1) { + rc = amr_parse_mr_conf(&lchan->tch.amr_mr, + TLVP_VAL(&tp, RSL_IE_MR_CONFIG), + TLVP_LEN(&tp, RSL_IE_MR_CONFIG)); + if (rc < 0) { LOGPLCHAN(lchan, DRSL, LOGL_ERROR, "Error parsing MultiRate conf IE\n"); rsl_tx_error_report(msg->trx, RSL_ERR_IE_CONTENT, &dch->chan_nr, NULL, msg); return rsl_tx_mode_modif_nack(lchan, RSL_ERR_IE_CONTENT);; } - memcpy(lchan->mr_bts_lv, TLVP_VAL(&tp, RSL_IE_MR_CONFIG) - 1, - TLVP_LEN(&tp, RSL_IE_MR_CONFIG) + 1); - amr_parse_mr_conf(&lchan->tch.amr_mr, TLVP_VAL(&tp, RSL_IE_MR_CONFIG), - TLVP_LEN(&tp, RSL_IE_MR_CONFIG)); - amr_log_mr_conf(DRTP, LOGL_DEBUG, gsm_lchan_name(lchan), - &lchan->tch.amr_mr); + + amr_log_mr_conf(DRTP, LOGL_DEBUG, gsm_lchan_name(lchan), &lchan->tch.amr_mr); lchan->tch.last_cmr = AMR_CMR_NONE; } /* 9.3.53 MultiRate Control */ -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/18680 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ibfd5845ea429945b352dd14421e86562998d65ca Gerrit-Change-Number: 18680 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-MessageType: merged