neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/32614 )
Change subject: rab_ass_fsm.c: fix asn1 memleak when replacing GTP address ......................................................................
rab_ass_fsm.c: fix asn1 memleak when replacing GTP address
This fix is really weird: by *removing* a FREE() call, fix a leak of the replaced transportLayerAddress. (An in-code comment says some more.)
Also remove the other FREE() call that turns out to not be necessary.
This has been verified with a ttcn3 test that is not yet submitted, which ensures an empty talloc_asn1_ctx at the end of every test (osmo-ttcn3-hacks I2948ee6f167369a2252f85b493e9653b93c7e4e9 ).
Change-Id: I315d04a07b7dfd4dce26e5b5f871318e27e2bdf6 --- M src/osmo-hnbgw/ps_rab_ass_fsm.c 1 file changed, 29 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/14/32614/1
diff --git a/src/osmo-hnbgw/ps_rab_ass_fsm.c b/src/osmo-hnbgw/ps_rab_ass_fsm.c index 464c4d0..2b7b940 100644 --- a/src/osmo-hnbgw/ps_rab_ass_fsm.c +++ b/src/osmo-hnbgw/ps_rab_ass_fsm.c @@ -547,7 +547,15 @@ return; }
- /* Replace GTP endpoint */ + /* Replace GTP endpoint. + * memory: ranap_new_transp_layer_addr() frees previous buffer in transportLayerAddress, if any. The + * entire rab_item is freed along with item_ies. + * + * BTW, If I free this explicitly below with + * ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_BIT_STRING, &rab_item->transportLayerAddress); + * that causes a leak in talloc_asn1_ctx: "iu_helpers.c:205 contains 20 bytes in 1 blocks". + * I couldn't figure out why, but things are freed properly when leaving it all up to item_ies. + */ if (ranap_new_transp_layer_addr(rab_item->transportLayerAddress, &rab->core.local.addr, rab->access.use_x213_nsap) < 0) { ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_RANAP_RAB_SetupOrModifiedItem, &item_ies); @@ -564,13 +572,13 @@
teid_be = htonl(rab->core.local.teid); rab_item->iuTransportAssociation->present = RANAP_IuTransportAssociation_PR_gTP_TEI; + /* memory: OCTET_STRING_fromBuf() frees previous buffer in gTP_TEI, if any. The entire rab_item is freed + * along with item_ies. */ OCTET_STRING_fromBuf(&rab_item->iuTransportAssociation->choice.gTP_TEI, (const char *)&teid_be, sizeof(teid_be));
/* Reencode this list item in the RANAP message */ rc = ANY_fromType_aper(&list_ie->value, &asn_DEF_RANAP_RAB_SetupOrModifiedItem, rab_item); - ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_BIT_STRING, &rab_item->transportLayerAddress); - ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_OCTET_STRING, &rab_item->iuTransportAssociation->choice.gTP_TEI); if (rc < 0) { ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_RANAP_RAB_SetupOrModifiedItem, &item_ies); LOG_PS_RAB_ASS(rab_ass, LOGL_ERROR, "Re-encoding RANAP PS RAB-AssignmentResponse failed\n");