[M] Change in osmo-hnbgw[master]: ranap_rab_ass: do not free never allocated FieldItems

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33390 ) Change subject: ranap_rab_ass: do not free never allocated FieldItems ...................................................................... ranap_rab_ass: do not free never allocated FieldItems When we check for an assignment failure in a RAB AssignmnetResponse, we use decode_rab_flitms_from_resp_ies to search through the list of RAB FailedItemsIEs for the RAB id we want to check. In case of failure, we get a positive index back from this function. We then know that the RAB that we checked for has failed bcause it was in the FailedItemsList. In that case, we also must free the RAB FailedItemsIEs that decode_rab_flitms_from_resp_ies has decoded. At the moment we also free the RAB FailedItemsIEs also when decode_rab_flitms_from_resp_ies returns with a negative return code, which may mean that the RAB was not found or some other error occured. In this case we must not free since then no valid RAB FailedItemsIEs are allocated. Related: OS#6062 Change-Id: I755ba6599079810a048bf5b6d8947b5a9ffa622d --- M src/osmo-hnbgw/ranap_rab_ass.c M tests/ranap_rab_ass/ranap_rab_ass_test.c 2 files changed, 55 insertions(+), 5 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/osmo-hnbgw/ranap_rab_ass.c b/src/osmo-hnbgw/ranap_rab_ass.c index cd8e625..e8addcd 100644 --- a/src/osmo-hnbgw/ranap_rab_ass.c +++ b/src/osmo-hnbgw/ranap_rab_ass.c @@ -176,7 +176,9 @@ return ies->raB_FailedList.raB_FailedList_ies.list.array[index]; } -/* find the RAB specified by rab_id in ies and when found, decode the result into items_ies */ +/* Find the RAB specified by rab_id in ies, decode the result into items_ies and return a positive index. + * The caller is responsible for freeing the contents in items_ies. In case of failure, the return code + * will be negative. */ static int decode_rab_smditms_from_resp_ies(RANAP_RAB_SetupOrModifiedItemIEs_t *items_ies, RANAP_RAB_AssignmentResponseIEs_t *ies, uint8_t rab_id) { @@ -576,17 +578,16 @@ RANAP_RAB_FailedItemIEs_t _rab_failed_items_ies; RANAP_RAB_FailedItemIEs_t *rab_failed_items_ies = &_rab_failed_items_ies; int rc; - bool result = true; - /* If we can get a failed item for the specified RAB ID, then we know that the + /* If we can get a failed item (rc >= 0) for the specified RAB ID, then we know that the * HNB reported the RAB Assignment as failed */ rc = decode_rab_flitms_from_resp_ies(rab_failed_items_ies, ies, rab_id); if (rc < 0) - result = false; + return false; ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_RANAP_RAB_FailedItem, rab_failed_items_ies); - return result; + return true; } /*! Check if a specific RAB is present in an RAB-ReleaseList inside RANAP_RAB_AssignmentRequestIEs. diff --git a/tests/ranap_rab_ass/ranap_rab_ass_test.c b/tests/ranap_rab_ass/ranap_rab_ass_test.c index fc2992d..fbdfa77 100644 --- a/tests/ranap_rab_ass/ranap_rab_ass_test.c +++ b/tests/ranap_rab_ass/ranap_rab_ass_test.c @@ -299,6 +299,31 @@ ranap_cn_rx_co_free(&message); } +/* Same as above but with a message that does not even contain a FailedItemsIEs list */ +void test_ranap_rab_ass_resp_ies_check_failure_no_faileditemies(void) +{ + int rc; + ranap_message message; + bool rab_failed_at_hnb; + uint8_t testvec[] = { + 0x60, 0x00, 0x00, 0x2a, 0x00, 0x00, 0x01, 0x00, + 0x34, 0x40, 0x23, 0x00, 0x00, 0x01, 0x00, 0x33, + 0x40, 0x1c, 0x60, 0x3a, 0x7c, 0x35, 0x00, 0x01, + 0x0a, 0x09, 0x01, 0xa4, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x40, 0x04, 0x0a, 0x00, 0x00 + }; + + rc = ranap_cn_rx_co_decode2(&message, testvec, sizeof(testvec)); + + OSMO_ASSERT(rc == 0); + rab_failed_at_hnb = + ranap_rab_ass_resp_ies_check_failure(&message.msg.raB_AssignmentResponseIEs, 23); + OSMO_ASSERT(rab_failed_at_hnb == 0) + + ranap_cn_rx_co_free(&message); +} + void test_ranap_rab_ass_req_ies_check_release(void) { int rc; @@ -413,6 +438,7 @@ test_ranap_rab_ass_req_ies_replace_inet_addr(); test_ranap_rab_ass_resp_ies_replace_inet_addr(); test_ranap_rab_ass_resp_ies_check_failure(); + test_ranap_rab_ass_resp_ies_check_failure_no_faileditemies(); test_ranap_rab_ass_req_ies_check_release(); test_ranap_rab_ass_req_ies_get_count(); -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33390 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I755ba6599079810a048bf5b6d8947b5a9ffa622d Gerrit-Change-Number: 33390 Gerrit-PatchSet: 2 Gerrit-Owner: dexter &lt;pmaier(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: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged