Fix ASN.1 memory management ownership in NGAP functions

In the current memory management the caller wasn't responsible for freeing
memory they allocate. The ngap_gNB_encode_pdu function was taking ownership
of PDU contents by calling ASN_STRUCT_FREE_CONTENTS_ONLY, while callers
also expected to manage the same memory. This created unclear ownership semantics and
potential double-free bugs.

- Remove ASN_STRUCT_FREE_CONTENTS_ONLY from ngap_gNB_encode_pdu to prevent double-free
- Remove unnecessary ASN_STRUCT_FREE when encode function returns NULL
- Add proper ASN_STRUCT_FREE cleanup for heap-allocated PDUs in ngap_gNB.c functions
- Add ASN_STRUCT_FREE_CONTENTS_ONLY cleanup for stack-allocated PDUs in NAS and context management functions
- Ensure consistent memory cleanup patterns across all NGAP encoder callers
- Fix potential memory leaks in all ngap_gNB_encode_pdu call sites

This prevents double-free bugs and ensures proper cleanup of ASN.1 structures
in both heap and stack allocation scenarios.
This commit is contained in:
Guido Casati
2025-09-30 11:48:53 +02:00
parent 524a3ddf85
commit 235493aa07
4 changed files with 34 additions and 9 deletions
+9 -7
View File
@@ -298,7 +298,6 @@ int ngap_handover_required(instance_t instance, ngap_handover_required_t *msg)
NGAP_NGAP_PDU_t *pdu = encode_ng_handover_required(msg);
if (!pdu) {
NGAP_ERROR("Failed to encode Handover Required\n");
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
return -1;
}
@@ -307,11 +306,10 @@ int ngap_handover_required(instance_t instance, ngap_handover_required_t *msg)
byte_array_t out = { .buf = NULL, .len = 0 };
if (ngap_gNB_encode_pdu(pdu, &out.buf, (uint32_t *)&out.len) < 0) {
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
NGAP_ERROR("Failed to encode Handover Required message\n");
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
return -1;
}
free(pdu);
/* UE associated signalling -> use the allocated stream */
ngap_gNB_itti_send_sctp_data_req(ngap_gNB_instance_p->instance,
@@ -320,6 +318,7 @@ int ngap_handover_required(instance_t instance, ngap_handover_required_t *msg)
out.len,
ue_context_p->tx_stream);
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
return 0;
}
@@ -342,7 +341,6 @@ static int ngap_gNB_handover_failure(instance_t instance, const ngap_handover_fa
NGAP_NGAP_PDU_t *pdu = encode_ng_handover_failure(msg);
if (!pdu) {
NGAP_ERROR("Failed to encode NG Handover Failure\n");
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
return -1;
}
@@ -364,7 +362,7 @@ static int ngap_gNB_handover_failure(instance_t instance, const ngap_handover_fa
}
ngap_gNB_itti_send_sctp_data_req(ngap_gNB_instance_p->instance, amf->assoc_id, buffer, length, amf->nextstream);
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, &pdu);
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
return 0;
}
@@ -410,7 +408,7 @@ static int ngap_gNB_handover_request_acknowledge(instance_t instance, ngap_hando
/* UE associated signalling -> use the allocated stream */
ngap_gNB_itti_send_sctp_data_req(ngap->instance, ue_context_p.amf_ref->assoc_id, ba.buf, ba.len, ue_context_p.tx_stream);
NGAP_INFO("Sent Handover Request Acknowledge to AMF\n");
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
return 0;
}
@@ -450,6 +448,7 @@ static int ngap_gNB_handover_notify(instance_t instance, const ngap_handover_not
ba.buf,
ba.len,
ue_context_p->tx_stream);
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
return 0;
}
@@ -489,7 +488,6 @@ static int ngap_gNB_handover_cancel(instance_t instance, const ngap_handover_can
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
return -1;
}
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
/* UE-associated signalling -> use the UE's allocated SCTP stream */
ngap_gNB_itti_send_sctp_data_req(ngap_gNB_instance_p->instance,
@@ -498,6 +496,7 @@ static int ngap_gNB_handover_cancel(instance_t instance, const ngap_handover_can
ba.len,
ue_context_p->tx_stream);
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
return 0;
}
@@ -530,6 +529,7 @@ int ngap_gNB_handle_ul_ran_status_transfer(instance_t instance, const ngap_ran_s
}
ngap_gNB_itti_send_sctp_data_req(instance, ue_context_p->amf_ref->assoc_id, out.buf, out.len, ue_context_p->tx_stream);
ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu);
return 0;
}
@@ -769,11 +769,13 @@ static int ngap_gNB_generate_ng_setup_request(
if (ngap_gNB_encode_pdu(&pdu, &buffer, &len) < 0) {
NGAP_ERROR("Failed to encode NG setup request\n");
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
/* Non UE-Associated signalling -> stream = 0 */
ngap_gNB_itti_send_sctp_data_req(instance_p->instance, ngap_amf_data_p->assoc_id, buffer, len, 0);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return ret;
}
@@ -111,6 +111,7 @@ int ngap_ue_context_release_complete(instance_t instance,
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
/* Encode procedure has failed... */
NGAP_ERROR("Failed to encode UE context release complete\n");
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
@@ -125,6 +126,7 @@ int ngap_ue_context_release_complete(instance_t instance,
ngap_gNB_ue_context_t *tmp = ngap_detach_ue_context(ue_release_complete_p->gNB_ue_ngap_id);
if (tmp)
free(tmp);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}
@@ -207,6 +209,7 @@ int ngap_ue_context_release_req(instance_t instance,
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
/* Encode procedure has failed... */
NGAP_ERROR("Failed to encode UE context release complete\n");
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
@@ -215,6 +218,7 @@ int ngap_ue_context_release_req(instance_t instance,
ue_context_p->amf_ref->assoc_id, buffer,
length, ue_context_p->tx_stream);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}
-1
View File
@@ -148,6 +148,5 @@ int ngap_gNB_encode_pdu(NGAP_NGAP_PDU_t *pdu, uint8_t **buffer, uint32_t *len)
NGAP_DEBUG("Unknown message outcome (%d) or not implemented", (int)pdu->present);
return -1;
}
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, pdu);
return ret;
}
+21 -1
View File
@@ -259,8 +259,11 @@ int ngap_gNB_handle_nas_first_req(instance_t instance, ngap_nas_first_req_t *UEf
ie->value.choice.UEContextRequest = NGAP_UEContextRequest_requested;
}
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0)
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
DevMessage("Failed to encode initial UE message\n");
return -1;
}
/* Update the current NGAP UE state */
ue_desc_p.ue_state = NGAP_UE_WAITING_CSR;
@@ -288,6 +291,7 @@ int ngap_gNB_handle_nas_first_req(instance_t instance, ngap_nas_first_req_t *UEf
/* Send encoded message over sctp */
ngap_gNB_itti_send_sctp_data_req(instance_p->instance, amf->assoc_id, buffer, length, ue_desc_p.tx_stream);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}
@@ -454,6 +458,7 @@ int ngap_gNB_nas_uplink(instance_t instance, ngap_uplink_nas_t *ngap_uplink_nas_
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
NGAP_ERROR("Failed to encode uplink NAS transport\n");
/* Encode procedure has failed... */
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
@@ -462,6 +467,7 @@ int ngap_gNB_nas_uplink(instance_t instance, ngap_uplink_nas_t *ngap_uplink_nas_
ue_context_p->amf_ref->assoc_id, buffer,
length, ue_context_p->tx_stream);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}
@@ -535,6 +541,7 @@ int ngap_gNB_nas_non_delivery_ind(instance_t instance,
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
NGAP_ERROR("Failed to encode NAS NON delivery indication\n");
/* Encode procedure has failed... */
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
@@ -543,6 +550,7 @@ int ngap_gNB_nas_non_delivery_ind(instance_t instance,
ue_context_p->amf_ref->assoc_id, buffer,
length, ue_context_p->tx_stream);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}
@@ -704,6 +712,7 @@ int ngap_gNB_initial_ctxt_resp(instance_t instance, ngap_initial_context_setup_r
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
NGAP_ERROR("Failed to encode InitialContextSetupResponse\n");
/* Encode procedure has failed... */
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
@@ -711,6 +720,7 @@ int ngap_gNB_initial_ctxt_resp(instance_t instance, ngap_initial_context_setup_r
LOG_I(NR_RRC,"Send message to sctp: NGAP_InitialContextSetupResponse\n");
ngap_gNB_itti_send_sctp_data_req(ngap_gNB_instance_p->instance, ue_context_p->amf_ref->assoc_id, buffer, length, ue_context_p->tx_stream);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}
@@ -785,6 +795,7 @@ int ngap_gNB_initial_ctxt_fail(instance_t instance, ngap_initial_context_setup_f
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
NGAP_ERROR("Failed to encode InitialContextSetupFailure\n");
/* Encode procedure has failed... */
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
/* UE associated signalling -> use the allocated stream */
@@ -794,6 +805,7 @@ int ngap_gNB_initial_ctxt_fail(instance_t instance, ngap_initial_context_setup_f
buffer,
length,
ue_context_p->tx_stream);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}
@@ -866,11 +878,13 @@ int ngap_gNB_ue_capabilities(instance_t instance, ngap_ue_cap_info_ind_t *ue_cap
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
/* Encode procedure has failed... */
NGAP_ERROR("Failed to encode UE radio capabilities indication\n");
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
/* UE associated signalling -> use the allocated stream */
ngap_gNB_itti_send_sctp_data_req(ngap_gNB_instance_p->instance, ue_context_p->amf_ref->assoc_id, buffer, length, ue_context_p->tx_stream);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}
@@ -990,11 +1004,13 @@ int ngap_gNB_pdusession_setup_resp(instance_t instance, ngap_pdusession_setup_re
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
NGAP_ERROR("Failed to encode uplink transport\n");
/* Encode procedure has failed... */
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
/* UE associated signalling -> use the allocated stream */
ngap_gNB_itti_send_sctp_data_req(ngap_gNB_instance_p->instance, ue_context_p->amf_ref->assoc_id, buffer, length, ue_context_p->tx_stream);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}
@@ -1128,12 +1144,14 @@ int ngap_gNB_pdusession_modify_resp(instance_t instance, ngap_pdusession_modify_
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
NGAP_ERROR("Failed to encode uplink transport\n");
/* Encode procedure has failed... */
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
/* UE associated signalling -> use the allocated stream */
ngap_gNB_itti_send_sctp_data_req(ngap_gNB_instance_p->instance, ue_context_p->amf_ref->assoc_id, buffer, length, ue_context_p->tx_stream);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}
//------------------------------------------------------------------------------
@@ -1201,6 +1219,7 @@ int ngap_gNB_pdusession_release_resp(instance_t instance, ngap_pdusession_releas
if (ngap_gNB_encode_pdu(&pdu, &buffer, &length) < 0) {
NGAP_ERROR("Failed to encode release response\n");
/* Encode procedure has failed... */
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return -1;
}
@@ -1211,6 +1230,7 @@ int ngap_gNB_pdusession_release_resp(instance_t instance, ngap_pdusession_releas
(uint64_t)ue_context_p->amf_ue_ngap_id,
pdusession_release_resp_p->nb_of_pdusessions_released);
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NGAP_NGAP_PDU, &pdu);
return 0;
}