neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/31016 ) Change subject: fix possible leak of ue_context on UE REGISTER error ...................................................................... fix possible leak of ue_context on UE REGISTER error Deallocate a recently allocated UE context if the UE REGISTER procedure fails internally, in two places. The UE REGISTER error is rather unlikely to happen in practice: it only occurs when encoding the HNBAP response fails, which only gets checked input and thus is unlikely to fail. The same code paths also possibly leak asn1c allocations -- leave those for another patch. Related: SYS#6297 Change-Id: Icf4b82f9a904d56332c567f7ccfb24231ee66270 --- M src/osmo-hnbgw/hnbgw_hnbap.c 1 file changed, 18 insertions(+), 4 deletions(-) Approvals: Jenkins Builder: Verified dexter: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/src/osmo-hnbgw/hnbgw_hnbap.c b/src/osmo-hnbgw/hnbgw_hnbap.c index 23d1f48..4aefe2b 100644 --- a/src/osmo-hnbgw/hnbgw_hnbap.c +++ b/src/osmo-hnbgw/hnbgw_hnbap.c @@ -284,6 +284,7 @@ uint32_t ctx_id; uint32_t tmsi = 0; struct ue_context *ue; + struct ue_context *ue_allocated = NULL; int rc; memset(&accept, 0, sizeof(accept)); @@ -331,14 +332,19 @@ ue = ue_context_by_tmsi(hnb->gw, tmsi); if (!ue) - ue = ue_context_alloc(hnb, NULL, tmsi); + ue = ue_allocated = ue_context_alloc(hnb, NULL, tmsi); asn1_u24_to_bitstring(&accept.context_ID, &ctx_id, ue->context_id); memset(&accept_out, 0, sizeof(accept_out)); rc = hnbap_encode_ueregisteraccepties(&accept_out, &accept); - if (rc < 0) + if (rc < 0) { + /* If we allocated the UE context but the UE REGISTER fails, get rid of it again: there will likely + * never be a UE DE-REGISTER for this UE from the HNB, and the ue_context would linger forever. */ + if (ue_allocated) + ue_context_free(ue_allocated); return rc; + } msg = hnbap_generate_successful_outcome(HNBAP_ProcedureCode_id_UERegister, HNBAP_Criticality_reject, @@ -475,6 +481,7 @@ { HNBAP_UERegisterRequestIEs_t ies; struct ue_context *ue; + struct ue_context *ue_allocated = NULL; char imsi[16]; int rc; @@ -516,11 +523,18 @@ ue = ue_context_by_imsi(ctx->gw, imsi); if (!ue) - ue = ue_context_alloc(ctx, imsi, 0); + ue = ue_allocated = ue_context_alloc(ctx, imsi, 0); hnbap_free_ueregisterrequesties(&ies); /* Send UERegisterAccept */ - return hnbgw_tx_ue_register_acc(ue); + rc = hnbgw_tx_ue_register_acc(ue); + if (rc < 0) { + /* If we allocated the UE context but the UE REGISTER fails, get rid of it again: there will likely + * never be a UE DE-REGISTER for this UE from the HNB, and the ue_context would linger forever. */ + if (ue_allocated) + ue_context_free(ue_allocated); + } + return rc; } static int hnbgw_rx_ue_deregister(struct hnb_context *ctx, ANY_t *in) -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31016 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Icf4b82f9a904d56332c567f7ccfb24231ee66270 Gerrit-Change-Number: 31016 Gerrit-PatchSet: 1 Gerrit-Owner: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter &lt;pmaier(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-CC: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: merged