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 <nhofm...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pma...@sysmocom.de>
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
Gerrit-CC: pespin <pes...@sysmocom.de>
Gerrit-MessageType: merged
