Change in osmo-bsc[master]: bssap: always check return value of tlv_parse()

fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27864 ) Change subject: bssap: always check return value of tlv_parse() ...................................................................... bssap: always check return value of tlv_parse() Also take a chance to replace tlv_parse() with osmo_bssap_tlv_parse(). Change-Id: I90a732d26b4e674d9f7f10197105f7bf9860261d --- M src/osmo-bsc/osmo_bsc_bssap.c 1 file changed, 34 insertions(+), 14 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c index 7f8c8a1..6e78f47 100644 --- a/src/osmo-bsc/osmo_bsc_bssap.c +++ b/src/osmo-bsc/osmo_bsc_bssap.c @@ -63,7 +63,7 @@ int rc; bool old_value = msc->remote_supports_osmux; - rc = tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l4h + 1, length - 1, 0, 0); + rc = osmo_bssap_tlv_parse(&tp, msg->l4h + 1, length - 1); if (rc < 0) LOGP(DMSC, LOGL_NOTICE, "Failed parsing TLV looking for Osmux support\n"); @@ -270,7 +270,10 @@ .tmsi = GSM_RESERVED_TMSI, }; - tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l4h + 1, payload_length - 1, 0, 0); + if (osmo_bssap_tlv_parse(&tp, msg->l4h + 1, payload_length - 1) < 0) { + LOGP(DMSC, LOGL_ERROR, "%s(): tlv_parse() failed\n", __func__); + return -1; + } remain = payload_length - 1; if (!TLVP_PRESENT(&tp, GSM0808_IE_IMSI)) { @@ -417,7 +420,10 @@ struct tlv_parsed tp; enum gsm0808_cause cause_0808; - tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l4h + 1, length - 1, 0, 0); + if (osmo_bssap_tlv_parse(&tp, msg->l4h + 1, length - 1) < 0) { + LOGPFSML(conn->fi, LOGL_ERROR, "%s(): tlv_parse() failed\n", __func__); + return -1; + } cause_0808 = gsm0808_get_cause(&tp); if (cause_0808 < 0) { @@ -476,7 +482,11 @@ conn->ciphering_handled = 1; - tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l4h + 1, payload_length - 1, 0, 0); + if (osmo_bssap_tlv_parse(&tp, msg->l4h + 1, payload_length - 1) < 0) { + LOGPFSML(conn->fi, LOGL_ERROR, "%s(): tlv_parse() failed\n", __func__); + return -1; + } + if (!TLVP_PRESENT(&tp, GSM0808_IE_ENCRYPTION_INFORMATION)) { LOGP(DMSC, LOGL_ERROR, "IE Encryption Information missing.\n"); reject_cause = GSM0808_CAUSE_INFORMATION_ELEMENT_OR_FIELD_MISSING; @@ -624,16 +634,14 @@ struct msgb *resp; struct tlv_parsed tp; const uint8_t *config, *control; - int rc; OSMO_ASSERT(conn); - rc = tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l4h + 1, length - 1, 0, 0); - if (rc < 0) { - LOGPFSML(conn->fi, LOGL_ERROR, "Error parsing TLVs of LCLS CONNT CTRL: %s\n", - msgb_hexdump(msg)); - return rc; + if (osmo_bssap_tlv_parse(&tp, msg->l4h + 1, length - 1) < 0) { + LOGPFSML(conn->fi, LOGL_ERROR, "%s(): tlv_parse() failed\n", __func__); + return -1; } + config = TLVP_VAL_MINLEN(&tp, GSM0808_IE_LCLS_CONFIG, 1); control = TLVP_VAL_MINLEN(&tp, GSM0808_IE_LCLS_CONN_STATUS_CTRL, 1); @@ -816,7 +824,10 @@ aoip = gscon_is_aoip(conn); - tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l4h + 1, length - 1, 0, 0); + if (osmo_bssap_tlv_parse(&tp, msg->l4h + 1, length - 1) < 0) { + LOGPFSML(conn->fi, LOGL_ERROR, "%s(): tlv_parse() failed\n", __func__); + return -1; + } /* Check for channel type element, if its missing, immediately reject */ if (!TLVP_PRESENT(&tp, GSM0808_IE_CHANNEL_TYPE)) { @@ -1019,7 +1030,10 @@ return -EINVAL; } - tlv_parse(&tp, gsm0808_att_tlvdef(), msg->l4h + 1, length - 1, 0, 0); + if (osmo_bssap_tlv_parse(&tp, msg->l4h + 1, length - 1) < 0) { + LOGPFSML(conn->fi, LOGL_ERROR, "%s(): tlv_parse() failed\n", __func__); + return -1; + } /* Check for channel type element, if its missing, immediately reject */ if (!TLVP_PRESENT(&tp, GSM0808_IE_LAYER_3_INFORMATION)) { @@ -1057,7 +1071,10 @@ enum gsm0808_cause_class cause_class; struct gsm0808_diagnostics *diag; - osmo_bssap_tlv_parse(&tp, msg->l4h + 1, length - 1); + if (osmo_bssap_tlv_parse(&tp, msg->l4h + 1, length - 1) < 0) { + LOGPFSML(conn->fi, LOGL_ERROR, "%s(): tlv_parse() failed\n", __func__); + return -1; + } /* Check for the Cause and Diagnostic mandatory elements */ if (!TLVP_PRESENT(&tp, GSM0808_IE_CAUSE) || !TLVP_PRESENT(&tp, GSM0808_IE_DIAGNOSTIC)) { @@ -1100,7 +1117,10 @@ { struct tlv_parsed tp; - osmo_bssap_tlv_parse(&tp, msg->l4h + 1, length - 1); + if (osmo_bssap_tlv_parse(&tp, msg->l4h + 1, length - 1) < 0) { + LOGPFSML(conn->fi, LOGL_ERROR, "%s(): tlv_parse() failed\n", __func__); + return -1; + } /* Check for the mandatory elements */ if (!TLVP_PRESENT(&tp, GSM0808_IE_IMSI)) { 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27864 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I90a732d26b4e674d9f7f10197105f7bf9860261d Gerrit-Change-Number: 27864 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged