From a7257eaff1341611356a96c7e51a8395b3463f2e Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Thu, 18 Sep 2025 22:12:37 +0200 Subject: [PATCH 01/26] E1 lib: fix inconsistent equality checking patterns in E1AP bearer context management Replace result &= pattern with consistent early return pattern: - Fix mixing of logical (&&) and binary (&) operators - Use consistent 'if (!eq_()) return false;' pattern throughout - Fix bug in eq_bearer_context_mod_response where drbModA was compared with itself - Remove unused 'bool result = true;' variables - Always return true at the end of equality functions This makes the code more consistent with the rest of the codebase and provides better error reporting through the existing _E1_EQ_CHECK macros. --- .../E1AP/lib/e1ap_bearer_context_management.c | 52 +++++++++++-------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/openair2/E1AP/lib/e1ap_bearer_context_management.c b/openair2/E1AP/lib/e1ap_bearer_context_management.c index 0f2e5972e2..fdccd94925 100644 --- a/openair2/E1AP/lib/e1ap_bearer_context_management.c +++ b/openair2/E1AP/lib/e1ap_bearer_context_management.c @@ -398,7 +398,6 @@ static bool eq_pdcp_info(const e1_pdcp_status_info_t *a, const e1_pdcp_status_in */ static bool eq_drb_to_setup(const DRB_nGRAN_to_setup_t *a, const DRB_nGRAN_to_setup_t *b) { - bool result = true; _E1_EQ_CHECK_LONG(a->id, b->id); _E1_EQ_CHECK_INT(a->numCellGroups, b->numCellGroups); for (int i = 0; i < a->numCellGroups; i++) { @@ -406,11 +405,14 @@ static bool eq_drb_to_setup(const DRB_nGRAN_to_setup_t *a, const DRB_nGRAN_to_se } _E1_EQ_CHECK_INT(a->numQosFlow2Setup, b->numQosFlow2Setup); for (int i = 0; i < a->numQosFlow2Setup; i++) { - result &= eq_qos_flow(&a->qosFlows[i], &b->qosFlows[i]); + if (!eq_qos_flow(&a->qosFlows[i], &b->qosFlows[i])) + return false; } - result &= eq_pdcp_config(&a->pdcp_config, &b->pdcp_config); - result &= eq_sdap_config(&a->sdap_config, &b->sdap_config); - return result; + if (!eq_pdcp_config(&a->pdcp_config, &b->pdcp_config)) + return false; + if (!eq_sdap_config(&a->sdap_config, &b->sdap_config)) + return false; + return true; } static void free_drb_to_setup_item(const DRB_nGRAN_to_setup_t *msg) @@ -1939,25 +1941,32 @@ static bool eq_security_ind(security_indication_t *a, security_indication_t *b) */ static bool eq_drb_to_mod(const DRB_nGRAN_to_mod_t *a, const DRB_nGRAN_to_mod_t *b) { - bool result = true; _E1_EQ_CHECK_LONG(a->id, b->id); _E1_EQ_CHECK_INT(a->numQosFlow2Setup, b->numQosFlow2Setup); for (int i = 0; i < a->numQosFlow2Setup; i++) { - result &= eq_qos_flow(&a->qosFlows[i], &b->qosFlows[i]); + if (!eq_qos_flow(&a->qosFlows[i], &b->qosFlows[i])) + return false; } _E1_EQ_CHECK_INT(a->numDlUpParam, b->numDlUpParam); for (int i = 0; i < a->numDlUpParam; i++) { _E1_EQ_CHECK_INT(a->DlUpParamList[i].cell_group_id, b->DlUpParamList[i].cell_group_id); - result &= eq_up_tl_info(&a->DlUpParamList[i].tl_info, &b->DlUpParamList[i].tl_info); + if (!eq_up_tl_info(&a->DlUpParamList[i].tl_info, &b->DlUpParamList[i].tl_info)) + return false; + } + if (a->pdcp_config && b->pdcp_config) { + if (!eq_pdcp_config(a->pdcp_config, b->pdcp_config)) + return false; + } + if (a->sdap_config && b->sdap_config) { + if (!eq_sdap_config(a->sdap_config, b->sdap_config)) + return false; } - if (a->pdcp_config && b->pdcp_config) - result &= eq_pdcp_config(a->pdcp_config, b->pdcp_config); - if (a->sdap_config && b->sdap_config) - result &= eq_sdap_config(a->sdap_config, b->sdap_config); _E1_EQ_CHECK_OPTIONAL_PTR(a, b, pdcp_status); - if (a->pdcp_status && b->pdcp_status) - result &= eq_pdcp_info(a->pdcp_status, b->pdcp_status); - return result; + if (a->pdcp_status && b->pdcp_status) { + if (!eq_pdcp_info(a->pdcp_status, b->pdcp_status)) + return false; + } + return true; } /** @brief Equality check for PDU session item to modify */ @@ -2423,8 +2432,6 @@ bool eq_bearer_context_mod_response(const e1ap_bearer_modif_resp_t *a, const e1a { if (!a || !b) return false; // Null-check both inputs - bool result = true; - // Basic members _E1_EQ_CHECK_INT(a->gNB_cu_cp_ue_id, b->gNB_cu_cp_ue_id); _E1_EQ_CHECK_INT(a->gNB_cu_up_ue_id, b->gNB_cu_up_ue_id); @@ -2440,8 +2447,10 @@ bool eq_bearer_context_mod_response(const e1ap_bearer_modif_resp_t *a, const e1a _E1_EQ_CHECK_OPTIONAL_IE(pduA, pduB, integrityProtectionIndication, _E1_EQ_CHECK_INT); _E1_EQ_CHECK_OPTIONAL_PTR(pduA, pduB, ng_DL_UP_TL_info); - if (pduA->ng_DL_UP_TL_info && pduB->ng_DL_UP_TL_info) - result &= eq_up_tl_info(pduA->ng_DL_UP_TL_info, pduB->ng_DL_UP_TL_info); + if (pduA->ng_DL_UP_TL_info && pduB->ng_DL_UP_TL_info) { + if (!eq_up_tl_info(pduA->ng_DL_UP_TL_info, pduB->ng_DL_UP_TL_info)) + return false; + } // DRB Modified List _E1_EQ_CHECK_INT(pduA->numDRBModified, pduB->numDRBModified); @@ -2455,7 +2464,8 @@ bool eq_bearer_context_mod_response(const e1ap_bearer_modif_resp_t *a, const e1a } _E1_EQ_CHECK_OPTIONAL_PTR(drbModA, drbModB, pdcp_status); if (drbModA->pdcp_status && drbModB->pdcp_status) { - result &= eq_pdcp_info(drbModA->pdcp_status, drbModA->pdcp_status); + if (!eq_pdcp_info(drbModA->pdcp_status, drbModB->pdcp_status)) + return false; } } @@ -2490,7 +2500,7 @@ bool eq_bearer_context_mod_response(const e1ap_bearer_modif_resp_t *a, const e1a } - return result; + return true; } /** From 8dcd253e60171bb0de622a49b55a5eb0735eb4e6 Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Thu, 18 Sep 2025 22:15:30 +0200 Subject: [PATCH 02/26] E1 lib: Adopt _E1_EQ_CHECK_OPTIONAL_PTR macro in E1AP bearer context management - Add _E1_EQ_CHECK_OPTIONAL_PTR checks for pdcp_config, sdap_config, UP_TL_information, and securityIndication - Fix bug in eq_pdu_session_to_mod_item where a->UP_TL_information was compared with itself instead of b->UP_TL_information - Fix eq_security_ind call to properly handle return value with early return pattern - Improve consistency with existing macro usage patterns in the codebase - Provide better error reporting for optional pointer field mismatches --- openair2/E1AP/lib/e1ap_bearer_context_management.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/openair2/E1AP/lib/e1ap_bearer_context_management.c b/openair2/E1AP/lib/e1ap_bearer_context_management.c index fdccd94925..109b0008d3 100644 --- a/openair2/E1AP/lib/e1ap_bearer_context_management.c +++ b/openair2/E1AP/lib/e1ap_bearer_context_management.c @@ -1953,10 +1953,12 @@ static bool eq_drb_to_mod(const DRB_nGRAN_to_mod_t *a, const DRB_nGRAN_to_mod_t if (!eq_up_tl_info(&a->DlUpParamList[i].tl_info, &b->DlUpParamList[i].tl_info)) return false; } + _E1_EQ_CHECK_OPTIONAL_PTR(a, b, pdcp_config); if (a->pdcp_config && b->pdcp_config) { if (!eq_pdcp_config(a->pdcp_config, b->pdcp_config)) return false; } + _E1_EQ_CHECK_OPTIONAL_PTR(a, b, sdap_config); if (a->sdap_config && b->sdap_config) { if (!eq_sdap_config(a->sdap_config, b->sdap_config)) return false; @@ -1973,7 +1975,8 @@ static bool eq_drb_to_mod(const DRB_nGRAN_to_mod_t *a, const DRB_nGRAN_to_mod_t static bool eq_pdu_session_to_mod_item(const pdu_session_to_mod_t *a, const pdu_session_to_mod_t *b) { _E1_EQ_CHECK_LONG(a->sessionId, b->sessionId); - if (a->UP_TL_information && a->UP_TL_information) { + _E1_EQ_CHECK_OPTIONAL_PTR(a, b, UP_TL_information); + if (a->UP_TL_information && b->UP_TL_information) { if (!eq_up_tl_info(a->UP_TL_information, b->UP_TL_information)) return false; } @@ -1987,8 +1990,10 @@ static bool eq_pdu_session_to_mod_item(const pdu_session_to_mod_t *a, const pdu_ if (!eq_drb_to_mod(&a->DRBnGRanModList[i], &b->DRBnGRanModList[i])) return false; } + _E1_EQ_CHECK_OPTIONAL_PTR(a, b, securityIndication); if (a->securityIndication && b->securityIndication) { - eq_security_ind(a->securityIndication, b->securityIndication); + if (!eq_security_ind(a->securityIndication, b->securityIndication)) + return false; } return true; } From 50760c04a705862246fe4e19cd7bae02ea8467df Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Thu, 18 Sep 2025 21:43:30 +0200 Subject: [PATCH 03/26] Fix log messages in N2 handover: print both PCI and Cell ID when available - Fixed incorrect log messages that said 'PCI=%lu' when printing nr_cell_id - Now prints both NR Cell ID and PCI when DU is found - Provides better debugging information for handover requests --- openair2/RRC/NR/rrc_gNB_NGAP.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openair2/RRC/NR/rrc_gNB_NGAP.c b/openair2/RRC/NR/rrc_gNB_NGAP.c index 34666e3538..75e6742422 100644 --- a/openair2/RRC/NR/rrc_gNB_NGAP.c +++ b/openair2/RRC/NR/rrc_gNB_NGAP.c @@ -1129,11 +1129,10 @@ void rrc_gNB_send_NGAP_HANDOVER_FAILURE(gNB_RRC_INST *rrc, ngap_handover_failure /** @brief Process NG Handover Request message (8.4.2.2 3GPP TS 38.413) */ int rrc_gNB_process_Handover_Request(gNB_RRC_INST *rrc, instance_t instance, ngap_handover_request_t *msg) { - LOG_I(NR_RRC, "Received Handover Request (on PCI=%lu) \n", msg->nr_cell_id); struct nr_rrc_du_container_t *du = get_du_by_cell_id(rrc, msg->nr_cell_id); if (du == NULL) { /* Cell Not Found! Return HO Request Failure*/ - LOG_E(RRC, "Failed to process Handover Request: no DU found with PCI=%lu \n", msg->nr_cell_id); + LOG_E(RRC, "Failed to process Handover Request: no DU found with NR Cell ID=%lu \n", msg->nr_cell_id); ngap_handover_failure_t fail = { .amf_ue_ngap_id = msg->amf_ue_ngap_id, .cause.type = NGAP_CAUSE_RADIO_NETWORK, @@ -1142,6 +1141,8 @@ int rrc_gNB_process_Handover_Request(gNB_RRC_INST *rrc, instance_t instance, nga rrc_gNB_send_NGAP_HANDOVER_FAILURE(rrc, &fail); return -1; } + uint16_t pci = du->setup_req->cell[0].info.nr_pci; + LOG_I(NR_RRC, "Received Handover Request (on NR Cell ID=%lu, PCI=%u) \n", msg->nr_cell_id, pci); // Create UE context sctp_assoc_t curr_assoc_id = du->assoc_id; From e535fbe6a715808fe1fee7f691ec77984467923c Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Tue, 23 Sep 2025 10:55:45 +0200 Subject: [PATCH 04/26] Replace impossible ID mismatch checks with DevAssert in NGAP functions - Replace error handling for impossible ID mismatches with DevAssert statements - If ngap_get_ue_context() succeeds, returned context must have matching NGAP gNB UE IDs by definition - This catches potential bugs in the lookup function immediately - Affects: handover_required, handover_notify, handover_cancel, ul_ran_status_transfer These checks were logically impossible since we looked up the context using the same IDs we were then checking for mismatch. --- openair3/NGAP/ngap_gNB.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/openair3/NGAP/ngap_gNB.c b/openair3/NGAP/ngap_gNB.c index 8c9ac6a973..46bcf93cad 100644 --- a/openair3/NGAP/ngap_gNB.c +++ b/openair3/NGAP/ngap_gNB.c @@ -293,12 +293,7 @@ int ngap_handover_required(instance_t instance, ngap_handover_required_t *msg) return -1; } - if ((ue_context_p->gNB_ue_ngap_id != msg->gNB_ue_ngap_id)) { - NGAP_ERROR("ue_context_p->gNB_ue_ngap_id %d does not match msg->gNB_ue_ngap_id %d\n", - ue_context_p->gNB_ue_ngap_id, - msg->gNB_ue_ngap_id); - return -1; - } + DevAssert(ue_context_p->gNB_ue_ngap_id == msg->gNB_ue_ngap_id); NGAP_NGAP_PDU_t *pdu = encode_ng_handover_required(msg); if (!pdu) { @@ -434,11 +429,7 @@ static int ngap_gNB_handover_notify(instance_t instance, const ngap_handover_not NGAP_ERROR("Failed to encode Handover Notify: no ue context associated with gNB_ue_ngap_id=%d\n", msg->gNB_ue_ngap_id); return -1; } - - if (ue_context_p->gNB_ue_ngap_id != msg->gNB_ue_ngap_id) { - NGAP_ERROR("Failed to encode Handover Notify: unknown gNB_ue_ngap_id=%d\n", msg->gNB_ue_ngap_id); - return -1; - } + DevAssert(ue_context_p->gNB_ue_ngap_id == msg->gNB_ue_ngap_id); if (ue_context_p->amf_ue_ngap_id != msg->amf_ue_ngap_id) { NGAP_ERROR("Failed to encode Handover Notify: unknown amf_ue_ngap_id=%ld\n", msg->amf_ue_ngap_id); @@ -479,11 +470,7 @@ static int ngap_gNB_handover_cancel(instance_t instance, const ngap_handover_can NGAP_ERROR("Failed to encode Handover Cancel: no UE context for gNB_ue_ngap_id=%d\n", msg->gNB_ue_ngap_id); return -1; } - - if (ue_context_p->gNB_ue_ngap_id != msg->gNB_ue_ngap_id) { - NGAP_ERROR("Failed to encode Handover Cancel: mismatched gNB_ue_ngap_id=%d\n", msg->gNB_ue_ngap_id); - return -1; - } + DevAssert(ue_context_p->gNB_ue_ngap_id == msg->gNB_ue_ngap_id); if (ue_context_p->amf_ue_ngap_id != msg->amf_ue_ngap_id) { NGAP_ERROR("Failed to encode Handover Cancel: mismatched amf_ue_ngap_id=%ld\n", msg->amf_ue_ngap_id); @@ -522,11 +509,13 @@ int ngap_gNB_handle_ul_ran_status_transfer(instance_t instance, const ngap_ran_s DevAssert(ngap_gNB_instance_p != NULL); ngap_gNB_ue_context_t *ue_context_p = ngap_get_ue_context(msg->gnb_ue_ngap_id); - if (!ue_context_p || ue_context_p->gNB_ue_ngap_id != msg->gnb_ue_ngap_id) { + if (!ue_context_p) { NGAP_ERROR("Could not find UE context for gNB_ue_ngap_id %d\n", msg->gnb_ue_ngap_id); return -1; } + DevAssert(ue_context_p->gNB_ue_ngap_id == msg->gnb_ue_ngap_id); + NGAP_NGAP_PDU_t *pdu = encode_ng_ul_ran_status_transfer(msg); if (!pdu) { NGAP_ERROR("Failed to encode UL RAN Status Transfer\n"); From b4485ecea2ff0fed61242c449ae61009d910b9df Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Thu, 18 Sep 2025 21:58:52 +0200 Subject: [PATCH 05/26] Fix UE context leak in handover failure - Add UE context cleanup before sending handover failure message - Prevents memory/resource leak when PDU session establishment fails during handover - UE context was created but never cleaned up on failure path This fixes a bug where UE contexts would accumulate over time when handovers failed after context creation. --- openair2/RRC/NR/rrc_gNB_NGAP.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openair2/RRC/NR/rrc_gNB_NGAP.c b/openair2/RRC/NR/rrc_gNB_NGAP.c index 75e6742422..a7ed703f7d 100644 --- a/openair2/RRC/NR/rrc_gNB_NGAP.c +++ b/openair2/RRC/NR/rrc_gNB_NGAP.c @@ -1196,12 +1196,15 @@ int rrc_gNB_process_Handover_Request(gNB_RRC_INST *rrc, instance_t instance, nga if (!trigger_bearer_setup(rrc, UE, msg->nb_of_pdusessions, to_setup, msg->ue_ambr.br_dl)) { LOG_E(NR_RRC, "Failed to establish PDU session: handover failed\n"); + ngap_handover_failure_t fail = { .amf_ue_ngap_id = msg->amf_ue_ngap_id, .cause.type = NGAP_CAUSE_RADIO_NETWORK, .cause.value = NGAP_CAUSE_RADIO_NETWORK_HO_FAILURE_IN_TARGET_5GC_NGRAN_NODE_OR_TARGET_SYSTEM, }; rrc_gNB_send_NGAP_HANDOVER_FAILURE(rrc, &fail); + rrc_remove_ue(rrc, ue_context_p); + return -1; } return 0; From a38aa1bab8cfab44942dbc324cea220201e31c74 Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Thu, 18 Sep 2025 22:02:47 +0200 Subject: [PATCH 06/26] Fix memory leaks in fill_removal_lists_from_source_measConfig - Add proper cleanup of allocated ASN.1 structures in all return paths - Fix early returns that were leaking NR_HandoverPreparationInformation - Fix early returns that were leaking NR_RRCReconfiguration - Add cleanup at end of successful function execution This prevents memory leaks when processing handover preparation information during N2 handover procedures. --- openair2/RRC/NR/MESSAGES/asn1_msg.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/openair2/RRC/NR/MESSAGES/asn1_msg.c b/openair2/RRC/NR/MESSAGES/asn1_msg.c index 0ed1750d6b..f76f3ee1d1 100644 --- a/openair2/RRC/NR/MESSAGES/asn1_msg.c +++ b/openair2/RRC/NR/MESSAGES/asn1_msg.c @@ -1231,6 +1231,7 @@ void fill_removal_lists_from_source_measConfig(NR_MeasConfig_t *currentMC, byte_ and extract the measConfig provided by the source gNB */ if (!hpi->criticalExtensions.choice.c1->choice.handoverPreparationInformation->sourceConfig) { LOG_W(NR_RRC, "Missing sourceConfig: in source gNB rrcReconfiguration\n"); + ASN_STRUCT_FREE(asn_DEF_NR_HandoverPreparationInformation, hpi); return; } NR_RRCReconfiguration_t *rrcReconf = NULL; @@ -1242,11 +1243,15 @@ void fill_removal_lists_from_source_measConfig(NR_MeasConfig_t *currentMC, byte_ sourceConfig->rrcReconfiguration.size); if (rrcReconf_dec_rval.code != RC_OK || rrcReconf_dec_rval.consumed < 0) { LOG_E(NR_RRC, "Failed to decode source gNB rrcReconfiguration!\n"); + ASN_STRUCT_FREE(asn_DEF_NR_HandoverPreparationInformation, hpi); return; } - if (rrcReconf->criticalExtensions.choice.rrcReconfiguration->measConfig == NULL) + if (rrcReconf->criticalExtensions.choice.rrcReconfiguration->measConfig == NULL) { + ASN_STRUCT_FREE(asn_DEF_NR_HandoverPreparationInformation, hpi); + ASN_STRUCT_FREE(asn_DEF_NR_RRCReconfiguration, rrcReconf); return; + } NR_MeasConfig_t *sourceMC = rrcReconf->criticalExtensions.choice.rrcReconfiguration->measConfig; @@ -1278,6 +1283,10 @@ void fill_removal_lists_from_source_measConfig(NR_MeasConfig_t *currentMC, byte_ asn1cSeqAdd(¤tMC->measIdToRemoveList->list, measId); } } + + // Clean up allocated memory + ASN_STRUCT_FREE(asn_DEF_NR_HandoverPreparationInformation, hpi); + ASN_STRUCT_FREE(asn_DEF_NR_RRCReconfiguration, rrcReconf); } int doRRCReconfiguration_from_HandoverCommand(byte_array_t *ba, const byte_array_t handoverCommand) From 12d92bc836a65b2f09f50d591564845a385b67e7 Mon Sep 17 00:00:00 2001 From: Robert Schmidt Date: Fri, 19 Sep 2025 10:33:25 +0200 Subject: [PATCH 07/26] Fix crash in get_du_for_ue when F1 UE data does not exist Add check for cu_exists_f1_ue_data() before calling cu_get_f1_ue_data() to prevent assertion failure when telnet command 'ci fetch_du_by_ue_id' is used for UEs that have been handed over via N2 (and no longer have F1 UE data). This fixes the crash: Assertion (ret == HASH_TABLE_OK && data != ((void *)0)) failed! In get_hashtable_data() /oai-ran/openair2/F1AP/f1ap_ids.c:36 element for ue_id 1 not found The function now returns NULL gracefully when F1 UE data is not available. Co-authored-by: Guido Casati --- openair2/RRC/NR/rrc_gNB_du.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openair2/RRC/NR/rrc_gNB_du.c b/openair2/RRC/NR/rrc_gNB_du.c index db1a9cd459..e104641edc 100644 --- a/openair2/RRC/NR/rrc_gNB_du.c +++ b/openair2/RRC/NR/rrc_gNB_du.c @@ -619,6 +619,8 @@ void rrc_CU_process_f1_lost_connection(gNB_RRC_INST *rrc, f1ap_lost_connection_t nr_rrc_du_container_t *get_du_for_ue(gNB_RRC_INST *rrc, uint32_t ue_id) { + if (!cu_exists_f1_ue_data(ue_id)) + return NULL; f1_ue_data_t ue_data = cu_get_f1_ue_data(ue_id); return get_du_by_assoc_id(rrc, ue_data.du_assoc_id); } From 524a3ddf85412d3750db6db08e9c0f8806cd094a Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Fri, 19 Sep 2025 15:48:09 +0200 Subject: [PATCH 08/26] Fix xer_fprint parameter in ngap_handover_required Fix incorrect xer_fprint parameter - should pass pdu instead of &pdu to properly print ASN.1 structure for debugging. --- openair3/NGAP/ngap_gNB.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openair3/NGAP/ngap_gNB.c b/openair3/NGAP/ngap_gNB.c index 46bcf93cad..4331f130a9 100644 --- a/openair3/NGAP/ngap_gNB.c +++ b/openair3/NGAP/ngap_gNB.c @@ -303,7 +303,7 @@ int ngap_handover_required(instance_t instance, ngap_handover_required_t *msg) } if (LOG_DEBUGFLAG(DEBUG_ASN1)) - xer_fprint(stdout, &asn_DEF_NGAP_NGAP_PDU, &pdu); + xer_fprint(stdout, &asn_DEF_NGAP_NGAP_PDU, pdu); byte_array_t out = { .buf = NULL, .len = 0 }; if (ngap_gNB_encode_pdu(pdu, &out.buf, (uint32_t *)&out.len) < 0) { From 235493aa07f87cdfb58c9580e55bb5cf86d5fa4d Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Fri, 19 Sep 2025 15:40:22 +0200 Subject: [PATCH 09/26] 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. --- openair3/NGAP/ngap_gNB.c | 16 ++++++++------ .../ngap_gNB_context_management_procedures.c | 4 ++++ openair3/NGAP/ngap_gNB_encoder.c | 1 - openair3/NGAP/ngap_gNB_nas_procedures.c | 22 ++++++++++++++++++- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/openair3/NGAP/ngap_gNB.c b/openair3/NGAP/ngap_gNB.c index 4331f130a9..b2ba38de52 100644 --- a/openair3/NGAP/ngap_gNB.c +++ b/openair3/NGAP/ngap_gNB.c @@ -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; } diff --git a/openair3/NGAP/ngap_gNB_context_management_procedures.c b/openair3/NGAP/ngap_gNB_context_management_procedures.c index 7abbec2592..7f114d5bf6 100644 --- a/openair3/NGAP/ngap_gNB_context_management_procedures.c +++ b/openair3/NGAP/ngap_gNB_context_management_procedures.c @@ -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; } diff --git a/openair3/NGAP/ngap_gNB_encoder.c b/openair3/NGAP/ngap_gNB_encoder.c index 7a7ce758e1..be3945c2f0 100644 --- a/openair3/NGAP/ngap_gNB_encoder.c +++ b/openair3/NGAP/ngap_gNB_encoder.c @@ -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; } diff --git a/openair3/NGAP/ngap_gNB_nas_procedures.c b/openair3/NGAP/ngap_gNB_nas_procedures.c index fe86d7c5e0..cff9cd0b72 100644 --- a/openair3/NGAP/ngap_gNB_nas_procedures.c +++ b/openair3/NGAP/ngap_gNB_nas_procedures.c @@ -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; } From de07668b0bb07ac2f542fdced61e6153de7f4bfc Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Fri, 19 Sep 2025 15:58:40 +0200 Subject: [PATCH 10/26] Fix potential memory leak in ngap_gNB_handover_request_acknowledge Move UE context creation and storage to after successful PDU encoding to prevent memory leaks if encoding fails. Also add proper PDU cleanup when AMF fetch fails and use consistent NULL check style. This ensures UE context is only stored when the operation is guaranteed to succeed, improving error handling and memory management. --- openair3/NGAP/ngap_gNB.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/openair3/NGAP/ngap_gNB.c b/openair3/NGAP/ngap_gNB.c index b2ba38de52..68e77d369c 100644 --- a/openair3/NGAP/ngap_gNB.c +++ b/openair3/NGAP/ngap_gNB.c @@ -376,20 +376,6 @@ static int ngap_gNB_handover_request_acknowledge(instance_t instance, ngap_hando DevAssert(msg != NULL); DevAssert(ngap != NULL); - // First, create and store NGAP UE context - ngap_gNB_ue_context_t ue_context_p = { - .amf_ref = ngap_gNB_get_AMF_from_instance(ngap), - .gNB_ue_ngap_id = msg->gNB_ue_ngap_id, - .amf_ue_ngap_id = msg->amf_ue_ngap_id, - .gNB_instance = ngap, - .ue_state = NGAP_UE_CONNECTED, - }; - if (ue_context_p.amf_ref == NULL) { - NGAP_ERROR("Failed to fetch AMF for current NGAP instance\n"); - return -1; - } - ngap_store_ue_context(&ue_context_p); - NGAP_NGAP_PDU_t *pdu = encode_ng_handover_request_ack(msg); if (!pdu) { NGAP_ERROR("Failed to encode NG Handover Request Acknowledge\n"); @@ -405,6 +391,21 @@ static int ngap_gNB_handover_request_acknowledge(instance_t instance, ngap_hando return -1; } + // Create and store NGAP UE context + ngap_gNB_ue_context_t ue_context_p = { + .amf_ref = ngap_gNB_get_AMF_from_instance(ngap), + .gNB_ue_ngap_id = msg->gNB_ue_ngap_id, + .amf_ue_ngap_id = msg->amf_ue_ngap_id, + .gNB_instance = ngap, + .ue_state = NGAP_UE_CONNECTED, + }; + if (!ue_context_p.amf_ref) { + NGAP_ERROR("Failed to fetch AMF for current NGAP instance\n"); + ASN_STRUCT_FREE(asn_DEF_NGAP_NGAP_PDU, pdu); + return -1; + } + ngap_store_ue_context(&ue_context_p); + /* 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"); From 13571587224ab11a191fc9751d02a868cfbce345 Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Fri, 19 Sep 2025 16:51:34 +0200 Subject: [PATCH 11/26] Fix hardcoded instance 0 bug in UE context release command Use the instance parameter instead of hardcoded 0 to access the correct RRC instance. Also clean up redundant variable initialization. --- openair2/RRC/NR/rrc_gNB_NGAP.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/openair2/RRC/NR/rrc_gNB_NGAP.c b/openair2/RRC/NR/rrc_gNB_NGAP.c index a7ed703f7d..95fbf72d5d 100644 --- a/openair2/RRC/NR/rrc_gNB_NGAP.c +++ b/openair2/RRC/NR/rrc_gNB_NGAP.c @@ -1306,9 +1306,8 @@ void rrc_gNB_free_Handover_Command(ngap_handover_command_t *msg) */ int rrc_gNB_process_NGAP_UE_CONTEXT_RELEASE_COMMAND(MessageDef *msg_p, instance_t instance) { - gNB_RRC_INST *rrc = RC.nrrrc[0]; - uint32_t gNB_ue_ngap_id = 0; - gNB_ue_ngap_id = NGAP_UE_CONTEXT_RELEASE_COMMAND(msg_p).gNB_ue_ngap_id; + gNB_RRC_INST *rrc = RC.nrrrc[instance]; + uint32_t gNB_ue_ngap_id = NGAP_UE_CONTEXT_RELEASE_COMMAND(msg_p).gNB_ue_ngap_id; rrc_gNB_ue_context_t *ue_context_p = rrc_gNB_get_ue_context(RC.nrrrc[instance], gNB_ue_ngap_id); if (ue_context_p == NULL) { From 59803e221f7a5cdb9fdca6bbbe154d8049c8d019 Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Mon, 22 Sep 2025 10:31:18 +0200 Subject: [PATCH 12/26] Fix duplicate handover request handling - Add early check for existing UE context by AMF UE NGAP ID at beginning of rrc_gNB_process_Handover_Request - Add dedicated function to get UE context by AMF UE NGAP ID: rrc_gNB_get_ue_context_by_amf_ue_ngap_id() - Send handover failure with appropriate cause if UE context already exists - Remove redundant ho_context NULL check that could never be true for newly created UE contexts - Prevents duplicate handover processing and resource leaks --- openair2/RRC/NR/rrc_gNB_NGAP.c | 17 +++++++++++++---- openair2/RRC/NR/rrc_gNB_UE_context.c | 12 ++++++++++++ openair2/RRC/NR/rrc_gNB_UE_context.h | 1 + 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/openair2/RRC/NR/rrc_gNB_NGAP.c b/openair2/RRC/NR/rrc_gNB_NGAP.c index 95fbf72d5d..4c5b504f2d 100644 --- a/openair2/RRC/NR/rrc_gNB_NGAP.c +++ b/openair2/RRC/NR/rrc_gNB_NGAP.c @@ -1129,6 +1129,19 @@ void rrc_gNB_send_NGAP_HANDOVER_FAILURE(gNB_RRC_INST *rrc, ngap_handover_failure /** @brief Process NG Handover Request message (8.4.2.2 3GPP TS 38.413) */ int rrc_gNB_process_Handover_Request(gNB_RRC_INST *rrc, instance_t instance, ngap_handover_request_t *msg) { + // Check if UE context already exists for this AMF UE NGAP ID + rrc_gNB_ue_context_t *existing_ue_context = rrc_gNB_get_ue_context_by_amf_ue_ngap_id(rrc, msg->amf_ue_ngap_id); + if (existing_ue_context != NULL) { + LOG_E(RRC, "UE context already exists for AMF UE NGAP ID %ld, cannot process handover request\n", msg->amf_ue_ngap_id); + ngap_handover_failure_t fail = { + .amf_ue_ngap_id = msg->amf_ue_ngap_id, + .cause.type = NGAP_CAUSE_RADIO_NETWORK, + .cause.value = NGAP_CAUSE_RADIO_NETWORK_HO_FAILURE_IN_TARGET_5GC_NGRAN_NODE_OR_TARGET_SYSTEM, + }; + rrc_gNB_send_NGAP_HANDOVER_FAILURE(rrc, &fail); + return -1; + } + struct nr_rrc_du_container_t *du = get_du_by_cell_id(rrc, msg->nr_cell_id); if (du == NULL) { /* Cell Not Found! Return HO Request Failure*/ @@ -1150,10 +1163,6 @@ int rrc_gNB_process_Handover_Request(gNB_RRC_INST *rrc, instance_t instance, nga gNB_RRC_UE_t *UE = &ue_context_p->ue_context; // allocate context for target - if (UE->ho_context != NULL) { - LOG_E(NR_RRC, "Ongoing handover for UE %d, cannot trigger new\n", UE->rrc_ue_id); - return -1; - } UE->ho_context = alloc_ho_ctx(HO_CTX_TARGET); UE->ho_context->target->ho_trigger = nr_rrc_trigger_n2_ho_target; diff --git a/openair2/RRC/NR/rrc_gNB_UE_context.c b/openair2/RRC/NR/rrc_gNB_UE_context.c index 412c5231cc..9a2c3fb6f6 100644 --- a/openair2/RRC/NR/rrc_gNB_UE_context.c +++ b/openair2/RRC/NR/rrc_gNB_UE_context.c @@ -122,6 +122,18 @@ rrc_gNB_ue_context_t *rrc_gNB_get_ue_context_by_rnti_any_du(gNB_RRC_INST *rrc_in return NULL; } +/** @brief Fetch UE Context by the unique AMF UE NGAP ID */ +rrc_gNB_ue_context_t *rrc_gNB_get_ue_context_by_amf_ue_ngap_id(gNB_RRC_INST *rrc_instance_pP, uint64_t amf_ue_ngap_id) +{ + rrc_gNB_ue_context_t *ue_context_p; + RB_FOREACH (ue_context_p, rrc_nr_ue_tree_s, &(rrc_instance_pP->rrc_ue_head)) { + if (ue_context_p->ue_context.amf_ue_ngap_id == amf_ue_ngap_id) { + return ue_context_p; + } + } + return NULL; +} + void rrc_gNB_free_mem_ue_context(rrc_gNB_ue_context_t *const ue_context_pP) //----------------------------------------------------------------------------- { diff --git a/openair2/RRC/NR/rrc_gNB_UE_context.h b/openair2/RRC/NR/rrc_gNB_UE_context.h index 9485672463..b8edd159c4 100644 --- a/openair2/RRC/NR/rrc_gNB_UE_context.h +++ b/openair2/RRC/NR/rrc_gNB_UE_context.h @@ -46,6 +46,7 @@ rrc_gNB_ue_context_t* rrc_gNB_allocate_new_ue_context(gNB_RRC_INST* rrc_instance rrc_gNB_ue_context_t* rrc_gNB_get_ue_context(gNB_RRC_INST* rrc_instance_pP, ue_id_t ue); rrc_gNB_ue_context_t* rrc_gNB_get_ue_context_by_rnti(gNB_RRC_INST* rrc_instance_pP, sctp_assoc_t assoc_id, rnti_t rntiP); rrc_gNB_ue_context_t* rrc_gNB_get_ue_context_by_rnti_any_du(gNB_RRC_INST* rrc_instance_pP, rnti_t rntiP); +rrc_gNB_ue_context_t* rrc_gNB_get_ue_context_by_amf_ue_ngap_id(gNB_RRC_INST* rrc_instance_pP, uint64_t amf_ue_ngap_id); void rrc_gNB_free_mem_ue_context(rrc_gNB_ue_context_t* const ue_context_pP); From 487de67ff8eb3b4f32ee03b6fc95e2c8b5fc984c Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Tue, 23 Sep 2025 14:52:37 +0200 Subject: [PATCH 13/26] Fix measurement configuration memory leak with stack allocation - Replace heap allocation with stack allocation for seq_arr_t structures - Use proper seq_arr_free() with free functions for ASN.1 structures - Remove unnecessary malloc/free calls for sequence containers - Improve memory safety and performance with automatic cleanup Fixes 128-byte memory leak: Direct leak of 128 byte(s) in 2 object(s) allocated from: /#0 0x7e7f39ab4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 /#1 0x56df8b07312d in SEQUENCE_decode_uper /home/guido/repo/openairinterface5g/develop/cmake_targets/ran_build/build/openair2/RRC/LTE/MESSAGES/constr_SEQUENCE_uper.c:37 /#2 0x56df8b056551 in uper_decode /home/guido/repo/openairinterface5g/develop/cmake_targets/ran_build/build/openair2/RRC/LTE/MESSAGES/uper_decoder.c:83 /#3 0x56df8a896b1b in get_nr_mtc /home/guido/repo/openairinterface5g/develop/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c:4216 /#4 0x56df8a82feb8 in ue_context_setup_request /home/guido/repo/openairinterface5g/develop/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c:635 /#5 0x56df8a66a298 in nr_initiate_handover /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB_mobility.c:218 /#6 0x56df8a6720af in nr_rrc_trigger_n2_ho_target /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB_mobility.c:526 /#7 0x56df8a5cf338 in rrc_gNB_process_e1_bearer_context_setup_resp /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB.c:2633 /#8 0x56df8a5d28ab in rrc_gnb_task /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB.c:3012 /#9 0x7e7f38494ac2 in start_thread nptl/pthread_create.c:442 --- openair2/RRC/NR/MESSAGES/asn1_msg.c | 6 +++++- openair2/RRC/NR/rrc_gNB.c | 28 +++++++++++++++++----------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/openair2/RRC/NR/MESSAGES/asn1_msg.c b/openair2/RRC/NR/MESSAGES/asn1_msg.c index f76f3ee1d1..d711217fbb 100644 --- a/openair2/RRC/NR/MESSAGES/asn1_msg.c +++ b/openair2/RRC/NR/MESSAGES/asn1_msg.c @@ -1121,7 +1121,11 @@ NR_MeasConfig_t *get_MeasConfig(const NR_MeasTiming_t *mt, if (rc_A3_seq) { for (int i = 0; i < rc_A3_seq->size; i++) { NR_ReportConfigToAddMod_t *rc_A3 = (NR_ReportConfigToAddMod_t *)seq_arr_at(rc_A3_seq, i); - asn1cSeqAdd(&mc->reportConfigToAddModList->list, rc_A3); + // Create a deep copy of the report config + NR_ReportConfigToAddMod_t *rc_A3_copy = NULL; + int result = asn_copy(&asn_DEF_NR_ReportConfigToAddMod, (void **)&rc_A3_copy, rc_A3); + AssertFatal(result >= 0, "error during asn_copy() of ReportConfigToAddMod\n"); + asn1cSeqAdd(&mc->reportConfigToAddModList->list, rc_A3_copy); } } diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c index cc766b9d81..2661531c27 100644 --- a/openair2/RRC/NR/rrc_gNB.c +++ b/openair2/RRC/NR/rrc_gNB.c @@ -638,11 +638,14 @@ NR_MeasConfig_t *nr_rrc_get_measconfig(const gNB_RRC_INST *rrc, uint64_t nr_cell nr_rrc_du_container_t *du = get_du_by_cell_id((gNB_RRC_INST *)rrc, nr_cellid); DevAssert(du != NULL); f1ap_served_cell_info_t *cell_info = &du->setup_req->cell[0].info; - NR_ReportConfigToAddMod_t *rc_PER = NULL; - NR_ReportConfigToAddMod_t *rc_A2 = NULL; - seq_arr_t *rc_A3_seq = NULL; - seq_arr_t *neigh_seq = NULL; + if (du->mtc != NULL) { + NR_ReportConfigToAddMod_t *rc_PER = NULL; + NR_ReportConfigToAddMod_t *rc_A2 = NULL; + seq_arr_t rc_A3_seq = {0}; + seq_arr_t neigh_seq = {0}; + seq_arr_init(&rc_A3_seq, sizeof(NR_ReportConfigToAddMod_t)); + seq_arr_init(&neigh_seq, sizeof(nr_neighbour_cell_t)); int scs = get_ssb_scs(cell_info); int band = get_dl_band(cell_info); const NR_MeasTimingList_t *mtlist = du->mtc->criticalExtensions.choice.c1->choice.measTimingConf->measTiming; @@ -657,21 +660,17 @@ NR_MeasConfig_t *nr_rrc_get_measconfig(const gNB_RRC_INST *rrc, uint64_t nr_cell If no related A3 but there is default add the default one. If default one added once as a report, no need to add it again && duplication. */ - rc_A3_seq = malloc(sizeof(seq_arr_t)); - neigh_seq = malloc(sizeof(seq_arr_t)); - seq_arr_init(rc_A3_seq, sizeof(NR_ReportConfigToAddMod_t)); - seq_arr_init(neigh_seq, sizeof(nr_neighbour_cell_t)); LOG_D(NR_RRC, "Preparing A3 Event Measurement Configuration!\n"); bool default_a3_added = false; // To ensure that the default configuration is only added once for (int i = 0; i < neighbour_cells->size; i++) { nr_neighbour_cell_t *neighbourCell = (nr_neighbour_cell_t *)seq_arr_at(neighbour_cells, i); if (default_a3_added && neighbourCell->physicalCellId == -1) continue; - seq_arr_push_back(neigh_seq, neighbourCell, sizeof(nr_neighbour_cell_t)); + seq_arr_push_back(&neigh_seq, neighbourCell, sizeof(nr_neighbour_cell_t)); const nr_a3_event_t *a3Event = get_a3_configuration((gNB_RRC_INST *)rrc, neighbourCell->physicalCellId); if (a3Event) { NR_ReportConfigId_t reportConfigId = neighbourCell->physicalCellId == -1 ? 3 : i + 4; - seq_arr_push_back(rc_A3_seq, prepare_a3_event_report(a3Event, reportConfigId), sizeof(NR_ReportConfigToAddMod_t)); + seq_arr_push_back(&rc_A3_seq, prepare_a3_event_report(a3Event, reportConfigId), sizeof(NR_ReportConfigToAddMod_t)); if (neighbourCell->physicalCellId == -1) default_a3_added = true; } @@ -681,7 +680,14 @@ NR_MeasConfig_t *nr_rrc_get_measconfig(const gNB_RRC_INST *rrc, uint64_t nr_cell rc_PER = prepare_periodic_event_report(rrc->measurementConfiguration.per_event); if (rrc->measurementConfiguration.a2_event) rc_A2 = prepare_a2_event_report(rrc->measurementConfiguration.a2_event); - return get_MeasConfig(mt, band, scs, cell_info->nr_pci, rc_PER, rc_A2, rc_A3_seq, neigh_seq); + + NR_MeasConfig_t *result = get_MeasConfig(mt, band, scs, cell_info->nr_pci, rc_PER, rc_A2, &rc_A3_seq, &neigh_seq); + + // Clean up sequence arrays + seq_arr_free(&rc_A3_seq, NULL); + seq_arr_free(&neigh_seq, NULL); + + return result; } return NULL; } From 915c41c127e48bd09c5905a5bc52246a60d7270c Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Tue, 23 Sep 2025 16:10:48 +0200 Subject: [PATCH 14/26] Fix MTC sequence memory leak in UE context setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add missing ASN_STRUCT_FREE for NR_MeasurementTimingConfiguration in ue_context_setup_request - Fix 64-byte MTC sequence leak in get_nr_mtc → SEQUENCE_decode_uper AddressSanitizer backtrace: Direct leak of 64 byte(s) in 1 object(s) allocated from: /#0 0x7dcfcb8b4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 /#1 0x591fcc59b67d in SEQUENCE_decode_uper /home/guido/repo/openairinterface5g/develop/cmake_targets/ran_build/build/openair2/RRC/LTE/MESSAGES/constr_SEQUENCE_uper.c:37 /#2 0x591fcc57eaa1 in uper_decode /home/guido/repo/openairinterface5g/develop/openair2/RRC/LTE/MESSAGES/uper_decoder.c:83 /#3 0x591fcbdbedeb in get_nr_mtc /home/guido/repo/openairinterface5g/develop/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c:4216 /#4 0x591fcbd58188 in ue_context_setup_request /home/guido/repo/openairinterface5g/develop/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c:635 /#5 0x591fcbb92568 in nr_initiate_handover /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB_mobility.c:218 /#6 0x591fcbb9a37f in nr_rrc_trigger_n2_ho_target /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB_mobility.c:526 /#7 0x591fcbaf7608 in rrc_gNB_process_e1_bearer_context_setup_resp /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB.c:2640 /#8 0x591fcbafab7b in rrc_gnb_task /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB.c:3019 /#9 0x7dcfca294ac2 in start_thread nptl/pthread_create.c:442 Root cause: ue_context_setup_request allocated NR_MeasurementTimingConfiguration via get_nr_mtc() but never freed it after creating the measurement gap configuration. The create_measgap_config function only reads from the structure and doesn't take ownership, so the caller must free it. --- openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c | 1 + 1 file changed, 1 insertion(+) diff --git a/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c b/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c index b3d596b524..debc0d5530 100644 --- a/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c +++ b/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c @@ -721,6 +721,7 @@ void ue_context_setup_request(const f1ap_ue_context_setup_req_t *req) /* free the memory we allocated above */ free_ue_context_setup_resp(&resp); ASN_STRUCT_FREE(asn_DEF_NR_CG_ConfigInfo, cg_configinfo); + ASN_STRUCT_FREE(asn_DEF_NR_MeasurementTimingConfiguration, mtc); } void ue_context_modification_request(const f1ap_ue_context_mod_req_t *req) From 5e3b4c497c54e9d31517b1873cca745ad8b86bfd Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Tue, 23 Sep 2025 16:46:41 +0200 Subject: [PATCH 15/26] Refactor handover context cleanup and fix memory leak in N2 handover source gNB path - Add missing handover context cleanup in rrc_CU_process_ue_context_release_complete - Fix 16-byte handover context leak in N2 handover source gNB trigger path AddressSanitizer backtrace: Direct leak of 16 byte(s) in 1 object(s) allocated from: /#0 0x7fee67cb4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 /#1 0x58059e33454b in calloc_or_fail /home/guido/repo/openairinterface5g/develop/common/utils/utils.h:74 /#2 0x58059e33454b in alloc_ho_ctx /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB_mobility.c:47 /#3 0x58059e339e4d in nr_rrc_trigger_n2_ho /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB_mobility.c:556 /#4 0x58059e33ae93 in nr_HO_N2_trigger_telnet /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB_mobility.c:611 Root cause: Handover context allocated in N2 handover source gNB trigger path (nr_rrc_trigger_n2_ho) for any successful N2 handover but not properly freed in all handover completion scenarios. The UE context release complete handler was missing cleanup for handover context. Solution: refactor handover context cleanup - Move nr_rrc_finalize_ho() call to rrc_delete_ue_data() for centralized cleanup - Remove redundant nr_rrc_finalize_ho() calls from handover failure paths: * rrc_CU_process_ue_context_release_request() - UE not deleted yet * invalidate_du_connections() - UE will be deleted later - Keep nr_rrc_finalize_ho() calls in handover success paths where UE continues operating - Ensures handover context is cleaned up exactly once when UE is actually deleted - Prevents potential double-free and use-after-free issues in handover cleanup This centralizes all UE data cleanup in rrc_delete_ue_data() and ensures handover context is properly managed throughout the UE lifecycle and cleaned up when UE delete happens. --- openair2/RRC/NR/rrc_gNB.c | 3 +++ openair2/RRC/NR/rrc_gNB_du.c | 4 +--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c index 2661531c27..aea6fb9259 100644 --- a/openair2/RRC/NR/rrc_gNB.c +++ b/openair2/RRC/NR/rrc_gNB.c @@ -2338,6 +2338,9 @@ static void rrc_CU_process_ue_context_release_request(MessageDef *msg_p, sctp_as static void rrc_delete_ue_data(gNB_RRC_UE_t *UE) { + /* Clean up handover context if it exists */ + if (UE->ho_context) + nr_rrc_finalize_ho(UE); ASN_STRUCT_FREE(asn_DEF_NR_UE_NR_Capability, UE->UE_Capability_nr); ASN_STRUCT_FREE(asn_DEF_NR_CellGroupConfig, UE->masterCellGroup); ASN_STRUCT_FREE(asn_DEF_NR_MeasResults, UE->measResults); diff --git a/openair2/RRC/NR/rrc_gNB_du.c b/openair2/RRC/NR/rrc_gNB_du.c index e104641edc..edf6f19164 100644 --- a/openair2/RRC/NR/rrc_gNB_du.c +++ b/openair2/RRC/NR/rrc_gNB_du.c @@ -453,10 +453,8 @@ static int invalidate_du_connections(gNB_RRC_INST *rrc, sctp_assoc_t assoc_id) RB_FOREACH(ue_context_p, rrc_nr_ue_tree_s, &rrc->rrc_ue_head) { gNB_RRC_UE_t *UE = &ue_context_p->ue_context; uint32_t ue_id = UE->rrc_ue_id; - if (UE->ho_context != NULL) { + if (UE->ho_context != NULL) LOG_W(NR_RRC, "DU disconnected while handover for UE %d active\n", ue_id); - nr_rrc_finalize_ho(UE); - } f1_ue_data_t ue_data = cu_get_f1_ue_data(ue_id); if (ue_data.du_assoc_id != assoc_id) continue; /* this UE is on another DU */ From feea066c3f1922e41c23859544d287fb7a11cf5e Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Tue, 23 Sep 2025 16:15:15 +0200 Subject: [PATCH 16/26] Fix measurement gap config memory leak in create_measgap_config - Add missing ASN_STRUCT_FREE for NR_MeasGapConfig in create_measgap_config - Fix 48-byte measurement gap config leak in get_gap_config_from_smtc AddressSanitizer backtrace: Direct leak of 48 byte(s) in 1 object(s) allocated from: /#0 0x7d32adab4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 /#1 0x5751f99cbb79 in calloc_or_fail /home/guido/repo/openairinterface5g/develop/common/utils/utils.h:74 /#2 0x5751f99cbb79 in get_gap_config_from_smtc /home/guido/repo/openairinterface5g/develop/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c:4164 /#3 0x5751f99cbb79 in create_measgap_config /home/guido/repo/openairinterface5g/develop/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c:4310 /#4 0x5751f9964abe in ue_context_setup_request /home/guido/repo/openairinterface5g/develop/openair2/LAYER2/NR_MAC_gNB/mac_rrc_dl_handler.c:709 /#5 0x5751f979e568 in nr_initiate_handover /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB_mobility.c:218 /#6 0x5751f97a637f in nr_rrc_trigger_n2_ho_target /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB_mobility.c:526 /#7 0x5751f9703608 in rrc_gNB_process_e1_bearer_context_setup_resp /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB.c:2641 /#8 0x5751f9706b7b in rrc_gnb_task /home/guido/repo/openairinterface5g/develop/openair2/RRC/NR/rrc_gNB.c:3020 /#9 0x7d32ac494ac2 in start_thread nptl/pthread_create.c:442 Root cause: create_measgap_config allocated NR_MeasGapConfig via get_gap_config_from_smtc() but never freed it after extracting the needed values to populate the measgap_config_t return structure. --- openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c b/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c index 44cb211b1d..8d6731dbbd 100644 --- a/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c +++ b/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c @@ -4361,8 +4361,10 @@ measgap_config_t create_measgap_config(const NR_MeasurementTimingConfiguration_t const NR_SSB_MTC_t *ssb_mtc = &ft->ssb_MeasurementTimingConfiguration; NR_MeasGapConfig_t *measGapConfig = get_gap_config_from_smtc(ssb_mtc); const NR_GapConfig_t *gap_config = get_gap_config(measGapConfig); - if (!gap_config) + if (!gap_config) { + ASN_STRUCT_FREE(asn_DEF_NR_MeasGapConfig, measGapConfig); return mgc; + } mgc.mgrp_ms = get_mgrp(gap_config->mgrp); DevAssert(mgc.mgrp_ms != -1); @@ -4385,6 +4387,10 @@ measgap_config_t create_measgap_config(const NR_MeasurementTimingConfiguration_t mgc.mgl_slots = ((int)(10 * (mgc.mgl_ms + max_k2)) << scs) / 10; mgc.enable = true; + + // Free the allocated measGapConfig structure after we're done using gap_config + ASN_STRUCT_FREE(asn_DEF_NR_MeasGapConfig, measGapConfig); + return mgc; } From c2e8743a7b7ce7a0590f8db7ac1aa3f8686370b4 Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Mon, 29 Sep 2025 17:34:15 +0200 Subject: [PATCH 17/26] Optimize create_measgap_config to eliminate unnecessary allocation - Replace inefficient 'allocate, extract, free' pattern with direct value extraction - Remove unused functions get_gap_config_from_smtc() and get_gap_config() - Add extract_gap_config_from_smtc() that extracts values directly without allocation - Eliminates use-after-free risk and improves performance - No allocation overhead, no memory leaks --- openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c | 126 ++++++++----------- 1 file changed, 49 insertions(+), 77 deletions(-) diff --git a/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c b/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c index 8d6731dbbd..a2b7c26678 100644 --- a/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c +++ b/openair2/LAYER2/NR_MAC_gNB/nr_radio_config.c @@ -50,6 +50,11 @@ #define NR_MAX_SUPPORTED_DL_LAYERS 4 +/* Default values for measurement gap configuration */ +#define DEFAULT_MGRP NR_GapConfig__mgrp_ms160 +#define DEFAULT_MGTA NR_GapConfig__mgta_ms0dot5 +#define DEFAULT_MGL NR_GapConfig__mgl_ms6 + #define PUCCH2_SIZE 8 const uint8_t slotsperframe[5] = {10, 20, 40, 80, 160}; @@ -4211,53 +4216,6 @@ NR_ReconfigurationWithSync_t *get_reconfiguration_with_sync(rnti_t rnti, uid_t u return reconfigurationWithSync; } -static NR_MeasGapConfig_t *get_gap_config_from_smtc(const NR_SSB_MTC_t *ssb_mtc) -{ - NR_MeasGapConfig_t *measGapConfig = calloc_or_fail(1, sizeof(*measGapConfig)); - measGapConfig->ext1 = calloc_or_fail(1, sizeof(*measGapConfig->ext1)); - measGapConfig->ext1->gapUE = calloc_or_fail(1, sizeof(*measGapConfig->ext1->gapUE)); - measGapConfig->ext1->gapUE->present = NR_SetupRelease_GapConfig_PR_setup; - NR_GapConfig_t *gap_config = calloc_or_fail(1, sizeof(*gap_config)); - measGapConfig->ext1->gapUE->choice.setup = gap_config; - - // mgta = Measurement Gap Timing Advance, to provide sufficient time for the UE to re-tune its transceiver - // This allows the Measurement Gap to extend mgta ms either side of the SS/PBCH Measurement Window. - gap_config->mgta = NR_GapConfig__mgta_ms0dot5; - - // mgrp = Measurement Gap Repetition Period - // gapOffset = It defines the start of the Measurement Gaps relative to the start of the radio frame with SFN = 0 - // The Measurement Gaps need to be synchronized with the SS/PBCH transmissions which are to be measured - gap_config->mgrp = NR_GapConfig__mgrp_ms160; - switch (ssb_mtc->periodicityAndOffset.present) { - case NR_SSB_MTC__periodicityAndOffset_PR_sf20: - gap_config->gapOffset = ssb_mtc->periodicityAndOffset.choice.sf20; - break; - case NR_SSB_MTC__periodicityAndOffset_PR_sf40: - gap_config->gapOffset = ssb_mtc->periodicityAndOffset.choice.sf40; - break; - case NR_SSB_MTC__periodicityAndOffset_PR_sf80: - gap_config->gapOffset = ssb_mtc->periodicityAndOffset.choice.sf80; - break; - case NR_SSB_MTC__periodicityAndOffset_PR_sf160: - gap_config->gapOffset = ssb_mtc->periodicityAndOffset.choice.sf160; - break; - default: - LOG_W(NR_RRC, "SMTC periodicity higher than MGRP\n"); - // With this configuration, not all SSBs belong to the Measurement Gap. - if (ssb_mtc->periodicityAndOffset.present == NR_SSB_MTC__periodicityAndOffset_PR_sf5) { - gap_config->gapOffset = ssb_mtc->periodicityAndOffset.choice.sf5; - } else if (ssb_mtc->periodicityAndOffset.present == NR_SSB_MTC__periodicityAndOffset_PR_sf10) { - gap_config->gapOffset = ssb_mtc->periodicityAndOffset.choice.sf10; - } - } - - // mgl = Measurement Gap Length - // FIXME: At least the duration of the SMTC plus 2 times the mgta should be enough, - // however, at the moment it only works by setting the maximum value - gap_config->mgl = NR_GapConfig__mgl_ms6; - - return measGapConfig; -} NR_MeasurementTimingConfiguration_t *get_nr_mtc(uint8_t *buf, uint32_t len) { @@ -4330,25 +4288,37 @@ static float get_mgta(long mgta) } } -/** @brief Return Measurement Gap Configuration, from ASN.1 config */ -const NR_GapConfig_t *get_gap_config(const NR_MeasGapConfig_t *mgc) +/** @brief Extract gapOffset from SSB MTC periodicity */ +static bool extract_gap_offset_from_smtc(const NR_SSB_MTC_t *ssb_mtc, long *gapOffset) { - if (mgc->gapFR2 && mgc->gapFR2->present == NR_SetupRelease_GapConfig_PR_setup) - return mgc->gapFR2->choice.setup; + if (!ssb_mtc) + return false; - // FR1 case - if (!mgc->ext1) - return NULL; + // Extract gapOffset based on periodicity + switch (ssb_mtc->periodicityAndOffset.present) { + case NR_SSB_MTC__periodicityAndOffset_PR_sf20: + *gapOffset = ssb_mtc->periodicityAndOffset.choice.sf20; + break; + case NR_SSB_MTC__periodicityAndOffset_PR_sf40: + *gapOffset = ssb_mtc->periodicityAndOffset.choice.sf40; + break; + case NR_SSB_MTC__periodicityAndOffset_PR_sf80: + *gapOffset = ssb_mtc->periodicityAndOffset.choice.sf80; + break; + case NR_SSB_MTC__periodicityAndOffset_PR_sf160: + *gapOffset = ssb_mtc->periodicityAndOffset.choice.sf160; + break; + default: + LOG_W(NR_RRC, "SMTC periodicity higher than MGRP\n"); + if (ssb_mtc->periodicityAndOffset.present == NR_SSB_MTC__periodicityAndOffset_PR_sf5) { + *gapOffset = ssb_mtc->periodicityAndOffset.choice.sf5; + } else if (ssb_mtc->periodicityAndOffset.present == NR_SSB_MTC__periodicityAndOffset_PR_sf10) { + *gapOffset = ssb_mtc->periodicityAndOffset.choice.sf10; + } + break; + } - const NR_SetupRelease_GapConfig_t *gapUE = mgc->ext1->gapUE; - if (gapUE && gapUE->present == NR_SetupRelease_GapConfig_PR_setup) - return gapUE->choice.setup; - - const NR_SetupRelease_GapConfig_t *gapFR1 = mgc->ext1->gapFR1; - if (gapFR1 && gapFR1->present == NR_SetupRelease_GapConfig_PR_setup) - return gapFR1->choice.setup; - - return NULL; + return true; } measgap_config_t create_measgap_config(const NR_MeasurementTimingConfiguration_t *mtc, int scs, int min_rxtxtime) @@ -4359,20 +4329,25 @@ measgap_config_t create_measgap_config(const NR_MeasurementTimingConfiguration_t DevAssert(mt != NULL && mt->frequencyAndTiming != NULL); const struct NR_MeasTiming__frequencyAndTiming *ft = mt->frequencyAndTiming; const NR_SSB_MTC_t *ssb_mtc = &ft->ssb_MeasurementTimingConfiguration; - NR_MeasGapConfig_t *measGapConfig = get_gap_config_from_smtc(ssb_mtc); - const NR_GapConfig_t *gap_config = get_gap_config(measGapConfig); - if (!gap_config) { - ASN_STRUCT_FREE(asn_DEF_NR_MeasGapConfig, measGapConfig); + + // Initialize with defaults + long mgrp = DEFAULT_MGRP; + long gapOffset = 0; + long mgta = DEFAULT_MGTA; + long mgl = DEFAULT_MGL; + + // Extract gapOffset from SSB MTC periodicity + if (!extract_gap_offset_from_smtc(ssb_mtc, &gapOffset)) { return mgc; } - mgc.mgrp_ms = get_mgrp(gap_config->mgrp); + mgc.mgrp_ms = get_mgrp(mgrp); DevAssert(mgc.mgrp_ms != -1); - mgc.mgrp = gap_config->mgrp; + mgc.mgrp = mgrp; - mgc.gapOffset = gap_config->gapOffset; - mgc.mgta = gap_config->mgta; - mgc.n_slots_mgta = ((int)(10 * get_mgta(gap_config->mgta)) << scs) / 10; + mgc.gapOffset = gapOffset; + mgc.mgta = mgta; + mgc.n_slots_mgta = ((int)(10 * get_mgta(mgta)) << scs) / 10; // We start the timer K2 slots earlier to avoid scheduling feedback PUCCHs inside measGap // or depending on the current min_rxtxtime earlier @@ -4381,16 +4356,13 @@ measgap_config_t create_measgap_config(const NR_MeasurementTimingConfiguration_t mgc.n_slots_advance = mgc.n_slots_mgta + max_k2; - mgc.mgl_ms = get_mgl(gap_config->mgl); + mgc.mgl_ms = get_mgl(mgl); DevAssert(mgc.mgl_ms != -1); - mgc.mgl = gap_config->mgl; + mgc.mgl = mgl; mgc.mgl_slots = ((int)(10 * (mgc.mgl_ms + max_k2)) << scs) / 10; mgc.enable = true; - // Free the allocated measGapConfig structure after we're done using gap_config - ASN_STRUCT_FREE(asn_DEF_NR_MeasGapConfig, measGapConfig); - return mgc; } From 3e8354727e03c3d8fce493732899943ca6630c3c Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Tue, 23 Sep 2025 14:15:44 +0200 Subject: [PATCH 18/26] Remove redundant NULL checks for handover encoder functions Handoer encoder functions never return NULL - they either return a valid pointer or terminate the program. Remove dead code paths. --- openair3/NGAP/ngap_gNB.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/openair3/NGAP/ngap_gNB.c b/openair3/NGAP/ngap_gNB.c index 68e77d369c..7cc8041ad5 100644 --- a/openair3/NGAP/ngap_gNB.c +++ b/openair3/NGAP/ngap_gNB.c @@ -296,10 +296,7 @@ int ngap_handover_required(instance_t instance, ngap_handover_required_t *msg) DevAssert(ue_context_p->gNB_ue_ngap_id == msg->gNB_ue_ngap_id); NGAP_NGAP_PDU_t *pdu = encode_ng_handover_required(msg); - if (!pdu) { - NGAP_ERROR("Failed to encode Handover Required\n"); - return -1; - } + DevAssert(pdu != NULL); if (LOG_DEBUGFLAG(DEBUG_ASN1)) xer_fprint(stdout, &asn_DEF_NGAP_NGAP_PDU, pdu); @@ -339,10 +336,7 @@ static int ngap_gNB_handover_failure(instance_t instance, const ngap_handover_fa DevAssert(ngap_gNB_instance_p != NULL); NGAP_NGAP_PDU_t *pdu = encode_ng_handover_failure(msg); - if (!pdu) { - NGAP_ERROR("Failed to encode NG Handover Failure\n"); - return -1; - } + DevAssert(pdu != NULL); if (ngap_gNB_encode_pdu(pdu, &buffer, &length) < 0) { NGAP_ERROR("Failed to encode HANDOVER FAILURE MESSAGE\n"); @@ -377,10 +371,7 @@ static int ngap_gNB_handover_request_acknowledge(instance_t instance, ngap_hando DevAssert(ngap != NULL); NGAP_NGAP_PDU_t *pdu = encode_ng_handover_request_ack(msg); - if (!pdu) { - NGAP_ERROR("Failed to encode NG Handover Request Acknowledge\n"); - return -1; - } + DevAssert(pdu != NULL); if (LOG_DEBUGFLAG(DEBUG_ASN1)) xer_fprint(stdout, &asn_DEF_NGAP_NGAP_PDU, pdu); @@ -436,6 +427,10 @@ static int ngap_gNB_handover_notify(instance_t instance, const ngap_handover_not } NGAP_NGAP_PDU_t *pdu = encode_ng_handover_notify(msg); + if (!pdu) { + NGAP_ERROR("Failed to encode NG Handover Notify\n"); + return -1; + } if (ngap_gNB_encode_pdu(pdu, &ba.buf, (uint32_t *)&ba.len) < 0) { NGAP_ERROR("Failed to encode NG Handover Notify message\n"); @@ -479,10 +474,7 @@ static int ngap_gNB_handover_cancel(instance_t instance, const ngap_handover_can /* Encode NGAP PDU */ NGAP_NGAP_PDU_t *pdu = encode_ng_handover_cancel(msg); - if (pdu == NULL) { - NGAP_ERROR("Failed to encode NG Handover Cancel\n"); - return -1; - } + DevAssert(pdu != NULL); if (ngap_gNB_encode_pdu(pdu, &ba.buf, (uint32_t *)&ba.len) < 0) { NGAP_ERROR("Failed to encode NG Handover Cancel message PDU\n"); @@ -517,10 +509,7 @@ int ngap_gNB_handle_ul_ran_status_transfer(instance_t instance, const ngap_ran_s DevAssert(ue_context_p->gNB_ue_ngap_id == msg->gnb_ue_ngap_id); NGAP_NGAP_PDU_t *pdu = encode_ng_ul_ran_status_transfer(msg); - if (!pdu) { - NGAP_ERROR("Failed to encode UL RAN Status Transfer\n"); - return -1; - } + DevAssert(pdu != NULL); byte_array_t out = {.buf = NULL, .len = 0}; if (ngap_gNB_encode_pdu(pdu, &out.buf, (uint32_t *)&out.len) < 0) { From 0cbed89b03c5201ae95184d0bf9519cada2d0569 Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Tue, 23 Sep 2025 14:20:39 +0200 Subject: [PATCH 19/26] Replace malloc_or_fail with calloc_or_fail for safer memory allocation - Update encoder functions in ngap_gNB_mobility_management.c to use calloc_or_fail - Provides zero-initialized memory and consistent error handling - Prevents uninitialized fields and improves memory safety Also: - Update NG Setup Request to use calloc_or_fail --- openair3/NGAP/ngap_gNB.c | 18 +++++++++--------- openair3/NGAP/ngap_gNB_mobility_management.c | 14 +++++++------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/openair3/NGAP/ngap_gNB.c b/openair3/NGAP/ngap_gNB.c index 7cc8041ad5..fb4d6c135b 100644 --- a/openair3/NGAP/ngap_gNB.c +++ b/openair3/NGAP/ngap_gNB.c @@ -677,7 +677,7 @@ static int ngap_gNB_generate_ng_setup_request( pdu.choice.initiatingMessage->value.present = NGAP_InitiatingMessage__value_PR_NGSetupRequest; out = &pdu.choice.initiatingMessage->value.choice.NGSetupRequest; /* mandatory */ - ie = (NGAP_NGSetupRequestIEs_t *)calloc(1, sizeof(NGAP_NGSetupRequestIEs_t)); + ie = calloc_or_fail(1, sizeof(NGAP_NGSetupRequestIEs_t)); ie->id = NGAP_ProtocolIE_ID_id_GlobalRANNodeID; ie->criticality = NGAP_Criticality_reject; ie->value.present = NGAP_NGSetupRequestIEs__value_PR_GlobalRANNodeID; @@ -700,7 +700,7 @@ static int ngap_gNB_generate_ng_setup_request( /* optional */ if (instance_p->gNB_name) { - ie = (NGAP_NGSetupRequestIEs_t *)calloc(1, sizeof(NGAP_NGSetupRequestIEs_t)); + ie = calloc_or_fail(1, sizeof(NGAP_NGSetupRequestIEs_t)); ie->id = NGAP_ProtocolIE_ID_id_RANNodeName; ie->criticality = NGAP_Criticality_ignore; ie->value.present = NGAP_NGSetupRequestIEs__value_PR_RANNodeName; @@ -710,28 +710,28 @@ static int ngap_gNB_generate_ng_setup_request( } /* mandatory */ - ie = (NGAP_NGSetupRequestIEs_t *)calloc(1, sizeof(NGAP_NGSetupRequestIEs_t)); + ie = calloc_or_fail(1, sizeof(NGAP_NGSetupRequestIEs_t)); ie->id = NGAP_ProtocolIE_ID_id_SupportedTAList; ie->criticality = NGAP_Criticality_reject; ie->value.present = NGAP_NGSetupRequestIEs__value_PR_SupportedTAList; { - ta = (NGAP_SupportedTAItem_t *)calloc(1, sizeof(NGAP_SupportedTAItem_t)); + ta = (NGAP_SupportedTAItem_t *)calloc_or_fail(1, sizeof(NGAP_SupportedTAItem_t)); INT24_TO_OCTET_STRING(instance_p->tac, &ta->tAC); { for (int i = 0; i < ngap_amf_data_p->broadcast_plmn_num; ++i) { - plmn = (NGAP_BroadcastPLMNItem_t *)calloc(1, sizeof(NGAP_BroadcastPLMNItem_t)); + plmn = (NGAP_BroadcastPLMNItem_t *)calloc_or_fail(1, sizeof(NGAP_BroadcastPLMNItem_t)); ngap_plmn_t *plmn_req = &instance_p->plmn[ngap_amf_data_p->broadcast_plmn_index[i]]; plmn_id_t *plmn_id = &instance_p->plmn[ngap_amf_data_p->broadcast_plmn_index[i]].plmn; MCC_MNC_TO_TBCD(plmn_id->mcc, plmn_id->mnc, plmn_id->mnc_digit_length, &plmn->pLMNIdentity); for (int si = 0; si < plmn_req->num_nssai; si++) { - ssi = (NGAP_SliceSupportItem_t *)calloc(1, sizeof(NGAP_SliceSupportItem_t)); + ssi = (NGAP_SliceSupportItem_t *)calloc_or_fail(1, sizeof(NGAP_SliceSupportItem_t)); INT8_TO_OCTET_STRING(plmn_req->s_nssai[si].sst, &ssi->s_NSSAI.sST); const uint32_t sd = plmn_req->s_nssai[si].sd; if (sd != 0xffffff) { - ssi->s_NSSAI.sD = calloc(1, sizeof(NGAP_SD_t)); - ssi->s_NSSAI.sD->buf = calloc(3, sizeof(uint8_t)); + ssi->s_NSSAI.sD = calloc_or_fail(1, sizeof(NGAP_SD_t)); + ssi->s_NSSAI.sD->buf = calloc_or_fail(3, sizeof(uint8_t)); ssi->s_NSSAI.sD->size = 3; ssi->s_NSSAI.sD->buf[0] = (sd & 0xff0000) >> 16; ssi->s_NSSAI.sD->buf[1] = (sd & 0x00ff00) >> 8; @@ -749,7 +749,7 @@ static int ngap_gNB_generate_ng_setup_request( asn1cSeqAdd(&out->protocolIEs.list, ie); /* mandatory */ - ie = (NGAP_NGSetupRequestIEs_t *)calloc(1, sizeof(NGAP_NGSetupRequestIEs_t)); + ie = calloc_or_fail(1, sizeof(NGAP_NGSetupRequestIEs_t)); ie->id = NGAP_ProtocolIE_ID_id_DefaultPagingDRX; ie->criticality = NGAP_Criticality_ignore; ie->value.present = NGAP_NGSetupRequestIEs__value_PR_PagingDRX; diff --git a/openair3/NGAP/ngap_gNB_mobility_management.c b/openair3/NGAP/ngap_gNB_mobility_management.c index a6ff4b39a1..95290f7bc3 100644 --- a/openair3/NGAP/ngap_gNB_mobility_management.c +++ b/openair3/NGAP/ngap_gNB_mobility_management.c @@ -37,7 +37,7 @@ * (9.2.3.1 of 3GPP TS 38.413) NG-RAN node → AMF */ NGAP_NGAP_PDU_t *encode_ng_handover_required(const ngap_handover_required_t *msg) { - NGAP_NGAP_PDU_t *pdu = malloc_or_fail(sizeof(*pdu)); + NGAP_NGAP_PDU_t *pdu = calloc_or_fail(1, sizeof(*pdu)); /* Prepare the NGAP message to encode */ pdu->present = NGAP_NGAP_PDU_PR_initiatingMessage; @@ -198,7 +198,7 @@ NGAP_NGAP_PDU_t *encode_ng_handover_required(const ngap_handover_required_t *msg NGAP_NGAP_PDU_t *encode_ng_handover_failure(const ngap_handover_failure_t *msg) { - NGAP_NGAP_PDU_t *pdu = malloc_or_fail(sizeof(*pdu)); + NGAP_NGAP_PDU_t *pdu = calloc_or_fail(1, sizeof(*pdu)); /* Prepare the NGAP message to encode */ pdu->present = NGAP_NGAP_PDU_PR_unsuccessfulOutcome; @@ -283,7 +283,7 @@ int decode_ng_handover_request(ngap_handover_request_t *out, const NGAP_NGAP_PDU // Mobility Restriction List (O) NGAP_FIND_PROTOCOLIE_BY_ID(NGAP_HandoverRequestIEs_t, ie, container, NGAP_ProtocolIE_ID_id_MobilityRestrictionList, false); if (ie != NULL) { - out->mobility_restriction = malloc_or_fail(sizeof(*out->mobility_restriction)); + out->mobility_restriction = calloc_or_fail(1, sizeof(*out->mobility_restriction)); *out->mobility_restriction = decode_ngap_mobility_restriction(&ie->value.choice.MobilityRestrictionList); } @@ -385,7 +385,7 @@ int decode_ng_handover_request(ngap_handover_request_t *out, const NGAP_NGAP_PDU NGAP_NGAP_PDU_t *encode_ng_handover_request_ack(ngap_handover_request_ack_t *msg) { - NGAP_NGAP_PDU_t *pdu = malloc_or_fail(sizeof(*pdu)); + NGAP_NGAP_PDU_t *pdu = calloc_or_fail(1, sizeof(*pdu)); pdu->present = NGAP_NGAP_PDU_PR_successfulOutcome; asn1cCalloc(pdu->choice.successfulOutcome, head); @@ -453,7 +453,7 @@ NGAP_NGAP_PDU_t *encode_ng_handover_request_ack(ngap_handover_request_ack_t *msg for (int j = 0; j < msg->pdusessions[pduSesIdx].ack_transfer.nb_of_qos_flow; j++) { asn1cSequenceAdd(transfer.qosFlowSetupResponseList.list, NGAP_QosFlowItemWithDataForwarding_t, qosItem); qosItem->qosFlowIdentifier = msg->pdusessions[pduSesIdx].ack_transfer.qos_setup_list[j].qfi; - qosItem->dataForwardingAccepted = malloc_or_fail(sizeof(*qosItem->dataForwardingAccepted)); + qosItem->dataForwardingAccepted = calloc_or_fail(1, sizeof(*qosItem->dataForwardingAccepted)); *qosItem->dataForwardingAccepted = NGAP_DataForwardingAccepted_data_forwarding_accepted; } @@ -565,7 +565,7 @@ void free_ng_handover_command(ngap_handover_command_t *msg) NGAP_NGAP_PDU_t *encode_ng_handover_notify(const ngap_handover_notify_t *msg) { - NGAP_NGAP_PDU_t *pdu = malloc_or_fail(sizeof(*pdu)); + NGAP_NGAP_PDU_t *pdu = calloc_or_fail(1, sizeof(*pdu)); // Message Type (M) pdu->present = NGAP_NGAP_PDU_PR_initiatingMessage; @@ -682,7 +682,7 @@ int decode_ng_handover_cancel_ack(ngap_handover_cancel_ack_t *out, const NGAP_NG /** @brief Encode NGAP UL RAN Status Transfer (9.2.3.14 of 3GPP TS 38.413) */ NGAP_NGAP_PDU_t *encode_ng_ul_ran_status_transfer(const ngap_ran_status_transfer_t *msg) { - NGAP_NGAP_PDU_t *pdu = malloc_or_fail(sizeof(*pdu)); + NGAP_NGAP_PDU_t *pdu = calloc_or_fail(1, sizeof(*pdu)); pdu->present = NGAP_NGAP_PDU_PR_initiatingMessage; asn1cCalloc(pdu->choice.initiatingMessage, head); head->procedureCode = NGAP_ProcedureCode_id_UplinkRANStatusTransfer; From c6a06dc81faeb866488cc51774337fc682ac7c5d Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Fri, 19 Sep 2025 15:48:22 +0200 Subject: [PATCH 20/26] Improve error message consistency in NGAP handover functions - ngap_gNB_handover_notify: Fix misleading error message from 'Failed to encode' to 'Failed to find UE context' for better clarity - ngap_gNB_handle_ul_ran_status_transfer: Standardize error message format from 'Could not find' to 'Failed to find' for consistency across functions This ensures consistent and accurate error reporting across all NGAP handover functions. --- openair3/NGAP/ngap_gNB.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openair3/NGAP/ngap_gNB.c b/openair3/NGAP/ngap_gNB.c index fb4d6c135b..b03e153486 100644 --- a/openair3/NGAP/ngap_gNB.c +++ b/openair3/NGAP/ngap_gNB.c @@ -416,7 +416,7 @@ static int ngap_gNB_handover_notify(instance_t instance, const ngap_handover_not DevAssert(ngap_gNB_instance_p != NULL); if ((ue_context_p = ngap_get_ue_context(msg->gNB_ue_ngap_id)) == NULL) { - NGAP_ERROR("Failed to encode Handover Notify: no ue context associated with gNB_ue_ngap_id=%d\n", msg->gNB_ue_ngap_id); + NGAP_ERROR("Failed to find UE context for Handover Notify: gNB_ue_ngap_id=%d\n", msg->gNB_ue_ngap_id); return -1; } DevAssert(ue_context_p->gNB_ue_ngap_id == msg->gNB_ue_ngap_id); @@ -502,7 +502,7 @@ int ngap_gNB_handle_ul_ran_status_transfer(instance_t instance, const ngap_ran_s ngap_gNB_ue_context_t *ue_context_p = ngap_get_ue_context(msg->gnb_ue_ngap_id); if (!ue_context_p) { - NGAP_ERROR("Could not find UE context for gNB_ue_ngap_id %d\n", msg->gnb_ue_ngap_id); + NGAP_ERROR("Failed to find UE context for gNB_ue_ngap_id %d\n", msg->gnb_ue_ngap_id); return -1; } From 1fc057cb08c141674040d73b9773a5a904abf9c9 Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Thu, 18 Sep 2025 22:09:51 +0200 Subject: [PATCH 21/26] Optimize PLMN identity assignment in handover procedures Replace field-by-field assignment with struct copy for plmn_identity. Both source and destination use identical plmn_id_t type, making this change safe and more maintainable. - Reduces lines of code - More readable and less error-prone - Single memory copy operation instead of multiple field assignments --- openair2/RRC/NR/rrc_gNB_NGAP.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/openair2/RRC/NR/rrc_gNB_NGAP.c b/openair2/RRC/NR/rrc_gNB_NGAP.c index 4c5b504f2d..b5915db574 100644 --- a/openair2/RRC/NR/rrc_gNB_NGAP.c +++ b/openair2/RRC/NR/rrc_gNB_NGAP.c @@ -1466,11 +1466,11 @@ void rrc_gNB_send_NGAP_HANDOVER_NOTIFY(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE) ho_notify->gNB_ue_ngap_id = UE->rrc_ue_id; ho_notify->amf_ue_ngap_id = UE->amf_ue_ngap_id; ho_notify->user_info.nrCellIdentity = rrc->nr_cellid; - ho_notify->user_info.target_ng_ran.tac = *du->setup_req->cell->info.tac; - ho_notify->user_info.target_ng_ran.targetgNBId = rrc->node_id; - ho_notify->user_info.target_ng_ran.plmn_identity.mcc = du->setup_req->cell->info.plmn.mcc; - ho_notify->user_info.target_ng_ran.plmn_identity.mnc = du->setup_req->cell->info.plmn.mnc; - ho_notify->user_info.target_ng_ran.plmn_identity.mnc_digit_length = du->setup_req->cell->info.plmn.mnc_digit_length; + + target_ran_node_id_t *target_ng_ran = &ho_notify->user_info.target_ng_ran; + target_ng_ran->tac = *du->setup_req->cell->info.tac; + target_ng_ran->targetgNBId = rrc->node_id; + target_ng_ran->plmn_identity = du->setup_req->cell->info.plmn; itti_send_msg_to_task(TASK_NGAP, rrc->module_id, msg_p); } @@ -1626,10 +1626,7 @@ void rrc_gNB_send_NGAP_HANDOVER_REQUIRED(gNB_RRC_INST *rrc, { LOG_I(NR_RRC, "Handover Preparation: send Handover Required (target gNB ID=%d, PCI=%d)\n", neighbour->gNB_ID, neighbour->physicalCellId); - const plmn_id_t plmn = {.mcc = neighbour->plmn.mnc, - .mnc = neighbour->plmn.mnc, - .mnc_digit_length = neighbour->plmn.mnc_digit_length}; - + const plmn_id_t plmn = neighbour->plmn; const target_ran_node_id_t target = { .plmn_identity = plmn, .tac = neighbour->tac, From 205bf7c993e59b91cea2eb4f81a5cd7289df46e9 Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Fri, 19 Sep 2025 16:44:54 +0200 Subject: [PATCH 22/26] Add NULL check for DU lookup in handover notify Add early return if get_du_by_cell_id returns NULL to prevent segmentation fault when accessing DU structure members. --- openair2/RRC/NR/rrc_gNB_NGAP.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/openair2/RRC/NR/rrc_gNB_NGAP.c b/openair2/RRC/NR/rrc_gNB_NGAP.c index b5915db574..8c839304c8 100644 --- a/openair2/RRC/NR/rrc_gNB_NGAP.c +++ b/openair2/RRC/NR/rrc_gNB_NGAP.c @@ -1457,11 +1457,15 @@ void rrc_gNB_send_NGAP_HANDOVER_REQUEST_ACKNOWLEDGE(gNB_RRC_INST *rrc, gNB_RRC_U void rrc_gNB_send_NGAP_HANDOVER_NOTIFY(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE) { LOG_I(NR_RRC, "Triggering NGAP Handover Notify\n"); + nr_rrc_du_container_t *du = get_du_by_cell_id(rrc, rrc->nr_cellid); + if (du == NULL) { + LOG_E(NR_RRC, "Failed to send Handover Notify: no DU found with NR Cell ID=%lu\n", rrc->nr_cellid); + return; + } MessageDef *msg_p = itti_alloc_new_message(TASK_RRC_GNB, 0, NGAP_HANDOVER_NOTIFY); ngap_handover_notify_t *ho_notify = &NGAP_HANDOVER_NOTIFY(msg_p); memset(ho_notify, 0, sizeof(*ho_notify)); - nr_rrc_du_container_t *du = get_du_by_cell_id(rrc, rrc->nr_cellid); ho_notify->gNB_ue_ngap_id = UE->rrc_ue_id; ho_notify->amf_ue_ngap_id = UE->amf_ue_ngap_id; From 9d9f272e74beba0dfa40c5056019610b90893b7c Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Fri, 19 Sep 2025 11:33:18 +0200 Subject: [PATCH 23/26] Refactor: split e1_send_bearer_updates into specialized functions Split monolithic e1_send_bearer_updates into three focused functions: - e1_send_bearer_updates(): DRB setup from F1 UE Context Modification Response - e1_request_pdcp_status(): Request PDCP status during inter-CU handover - e1_notify_pdcp_status(): Notify CU-UP with PDCP status during handover Extract common helpers: - append_e1_drb_mod_req(): Common DRB-to-mod for bearer request logic - e1_send_bearer_modification_request(): Common sending step Bug fixes: - Fix loop bounds (i < n instead of i == n) - Fix array access bounds checking - Fix DRB ID consistency Memory management: - Add missing free_e1ap_context_mod_request calls - Improve error handling with early returns In the process: - Simplify cuup_notify_reestablishment with FOR_EACH_SEQ_ARR loop - Adopted all callers to use new consolidated functions Improves maintainability and eliminates complex conditional logic. --- openair2/RRC/NR/nr_rrc_proto.h | 2 +- openair2/RRC/NR/rrc_gNB.c | 283 +++++++++++++++++++-------------- openair2/RRC/NR/rrc_gNB_NGAP.c | 2 +- 3 files changed, 166 insertions(+), 121 deletions(-) diff --git a/openair2/RRC/NR/nr_rrc_proto.h b/openair2/RRC/NR/nr_rrc_proto.h index fbd09d20a1..777ea9a8d1 100644 --- a/openair2/RRC/NR/nr_rrc_proto.h +++ b/openair2/RRC/NR/nr_rrc_proto.h @@ -127,6 +127,6 @@ void openair_rrc_gNB_configuration(gNB_RRC_INST *rrc, gNB_RrcConfigurationReq *c NR_SRB_ToAddModList_t *createSRBlist(gNB_RRC_UE_t *ue, uint8_t reestablish); NR_DRB_ToAddModList_t *createDRBlist(gNB_RRC_UE_t *ue, bool reestablish, bool do_integrity, bool do_ciphering); void activate_srb(gNB_RRC_UE_t *UE, int srb_id); -void e1_send_bearer_updates(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE, int n, f1ap_drb_setup_t *drbs, const ngap_drb_status_t *drb_status); +void e1_notify_pdcp_status(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE, const ngap_drb_status_t *drb_status); #endif diff --git a/openair2/RRC/NR/rrc_gNB.c b/openair2/RRC/NR/rrc_gNB.c index aea6fb9259..c2734c755a 100644 --- a/openair2/RRC/NR/rrc_gNB.c +++ b/openair2/RRC/NR/rrc_gNB.c @@ -879,22 +879,65 @@ static void fill_security_info(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE, security_inf nr_derive_key(UP_INT_ALG, secInfo->integrityProtectionAlgorithm, UE->kgnb, (uint8_t *)secInfo->integrityProtectionKey); } -/* \brief find existing PDU session inside E1AP Bearer Modif message, or - * point to new one. - * \param bearer_modif E1AP Bearer Modification Message - * \param pdu_id PDU session ID - * \return pointer to existing PDU session, or to new/unused one. */ -static pdu_session_to_mod_t *find_or_next_pdu_session(e1ap_bearer_mod_req_t *bearer_modif, int pdu_id) +/** @brief Add DRB modification to E1AP bearer modification request + * Finds PDU session by DRB ID, creates/updates PDU session modification entry + * and appends DRB modification + * @param req E1AP bearer modification request + * @param ue UE context + * @param drb_id DRB ID to find PDU session for + * @param drb_to_mod DRB modification data to append + * @return true if successful, false if PDU session not found */ +static bool append_e1_drb_mod_req(e1ap_bearer_mod_req_t *req, + gNB_RRC_UE_t *ue, + const int drb_id, + const DRB_nGRAN_to_mod_t *drb_to_mod) { - for (int i = 0; i < bearer_modif->numPDUSessionsMod; ++i) { - if (bearer_modif->pduSessionMod[i].sessionId == pdu_id) - return &bearer_modif->pduSessionMod[i]; + // Find PDU session by DRB ID + rrc_pdu_session_param_t *pdu = find_pduSession_from_drbId(ue, drb_id); + if (!pdu) { + LOG_E(NR_RRC, "UE %d: Failed to append E1 DRB mod, no PDU session found (DRB=%d)\n", ue->rrc_ue_id, drb_id); + return false; } - /* E1AP Bearer Modification has no PDU session to modify with that ID, create - * new entry */ - DevAssert(bearer_modif->numPDUSessionsMod < E1AP_MAX_NUM_PDU_SESSIONS - 1); - bearer_modif->numPDUSessionsMod += 1; - return &bearer_modif->pduSessionMod[bearer_modif->numPDUSessionsMod - 1]; + + int pdu_id = pdu->param.pdusession_id; + + // Find existing PDU session modification entry or create new one + pdu_session_to_mod_t *pdu_mod = NULL; + for (int i = 0; i < req->numPDUSessionsMod; ++i) { + if (req->pduSessionMod[i].sessionId == pdu_id) { + pdu_mod = &req->pduSessionMod[i]; + break; + } + } + + // Create new PDU session modification entry if not found + if (!pdu_mod) { + DevAssert(req->numPDUSessionsMod < E1AP_MAX_NUM_PDU_SESSIONS); + int new_index = req->numPDUSessionsMod++; + req->pduSessionMod[new_index].sessionId = pdu_id; + pdu_mod = &req->pduSessionMod[new_index]; + } + + // Append DRB modification + DevAssert(pdu_mod->numDRB2Modify < E1AP_MAX_NUM_DRBS); + pdu_mod->DRBnGRanModList[pdu_mod->numDRB2Modify++] = *drb_to_mod; + + return true; +} + +/** @brief Create DRB modification for reestablishment from existing DRB */ +static DRB_nGRAN_to_mod_t get_e1_drb_mod_reestablishment(const drb_t *drb, const bearer_context_pdcp_config_t *pdcp_config) +{ + DRB_nGRAN_to_mod_t drb_e1 = {0}; + drb_e1.id = drb->drb_id; + drb_e1.numDlUpParam = 1; + memcpy(&drb_e1.DlUpParamList[0].tl_info.tlAddress, &drb->du_tunnel_config.addr.buffer, sizeof(uint8_t) * 4); + drb_e1.DlUpParamList[0].tl_info.teId = drb->du_tunnel_config.teid; + /* PDCP configuration */ + drb_e1.pdcp_config = malloc_or_fail(sizeof(*drb_e1.pdcp_config)); + *drb_e1.pdcp_config = *pdcp_config; + drb_e1.pdcp_config->pDCP_Reestablishment = true; + return drb_e1; } /** @@ -907,44 +950,18 @@ static void cuup_notify_reestablishment(gNB_RRC_INST *rrc, gNB_RRC_UE_t *ue_p) .gNB_cu_up_ue_id = ue_p->rrc_ue_id, }; // Quit re-establishment notification if no CU-UP is associated - if (!is_cuup_associated(rrc)) { + if (!is_cuup_associated(rrc) || !ue_associated_to_cuup(rrc, ue_p)) { return; } - if (!ue_associated_to_cuup(rrc, ue_p)) - return; + + bool um_on_default_drb = rrc->configuration.um_on_default_drb; /* loop through active DRBs */ - for (int drb_id = 1; drb_id <= MAX_DRBS_PER_UE; drb_id++) { - drb_t *drb = get_drb(&ue_p->drbs, drb_id); - if (!drb) - continue; - /* fetch an existing PDU session for this DRB */ - rrc_pdu_session_param_t *pdu = find_pduSession_from_drbId(ue_p, drb_id); - if (pdu == NULL) { - LOG_E(RRC, "UE %d: E1 Bearer Context Modification: no PDU session for DRB ID %d\n", ue_p->rrc_ue_id, drb_id); - continue; + FOR_EACH_SEQ_ARR(drb_t *, drb, &ue_p->drbs) { + bearer_context_pdcp_config_t pdcp = set_bearer_context_pdcp_config(drb->pdcp_config, um_on_default_drb, ue_p->redcap_cap); + DRB_nGRAN_to_mod_t drb_e1 = get_e1_drb_mod_reestablishment(drb, &pdcp); + if (!append_e1_drb_mod_req(&req, ue_p, drb->drb_id, &drb_e1)) { + LOG_W(NR_RRC, "UE %d: Failed to append E1 DRB mod for reestablishment (DRB=%d)\n", ue_p->rrc_ue_id, drb->drb_id); } - /* Get pointer to existing (or new one) PDU session to modify in E1 */ - pdu_session_to_mod_t *pdu_e1 = find_or_next_pdu_session(&req, pdu->param.pdusession_id); - AssertError(pdu != NULL, - continue, - "UE %u: E1 Bearer Context Modification: PDU session %d to setup is null\n", - ue_p->rrc_ue_id, - pdu->param.pdusession_id); - /* Prepare PDU for E1 Bearear Context Modification Request */ - pdu_e1->sessionId = pdu->param.pdusession_id; - /* Fill DRB to setup with ID, DL TL and DL TEID */ - DRB_nGRAN_to_mod_t *drb_e1 = &pdu_e1->DRBnGRanModList[pdu_e1->numDRB2Modify]; - drb_e1->id = drb_id; - drb_e1->numDlUpParam = 1; - memcpy(&drb_e1->DlUpParamList[0].tl_info.tlAddress, &drb->du_tunnel_config.addr.buffer, sizeof(uint8_t) * 4); - drb_e1->DlUpParamList[0].tl_info.teId = drb->du_tunnel_config.teid; - /* PDCP configuration */ - if (!drb_e1->pdcp_config) - drb_e1->pdcp_config = malloc_or_fail(sizeof(*drb_e1->pdcp_config)); - *drb_e1->pdcp_config = set_bearer_context_pdcp_config(drb->pdcp_config, rrc->configuration.um_on_default_drb, ue_p->redcap_cap); - drb_e1->pdcp_config->pDCP_Reestablishment = true; - /* increase DRB to modify counter */ - pdu_e1->numDRB2Modify += 1; } #if 0 @@ -2094,12 +2111,14 @@ void rrc_gNB_process_dc_overall_timeout(const module_id_t gnb_mod_idP, x2ap_ENDC /* \brief fill E1 bearer modification's DRB from F1 DRB * \param drb_e1 pointer to a DRB inside an E1 bearer modification message * \param drb_f1 pointer to a DRB inside an F1 UE Ctxt modification Response */ -static void fill_e1_bearer_modif(DRB_nGRAN_to_mod_t *drb_e1, const f1ap_drb_setup_t *drb_f1) +static DRB_nGRAN_to_mod_t fill_e1_bearer_modif_from_f1(const f1ap_drb_setup_t *drb_f1) { - drb_e1->id = drb_f1->id; - drb_e1->numDlUpParam = drb_f1->up_dl_tnl_len; - drb_e1->DlUpParamList[0].tl_info.tlAddress = drb_f1->up_dl_tnl[0].tl_address; - drb_e1->DlUpParamList[0].tl_info.teId = drb_f1->up_dl_tnl[0].teid; + DRB_nGRAN_to_mod_t drb_e1 = {0}; + drb_e1.id = drb_f1->id; + drb_e1.numDlUpParam = drb_f1->up_dl_tnl_len; + drb_e1.DlUpParamList[0].tl_info.tlAddress = drb_f1->up_dl_tnl[0].tl_address; + drb_e1.DlUpParamList[0].tl_info.teId = drb_f1->up_dl_tnl[0].teid; + return drb_e1; } static gtpu_tunnel_t f1u_gtp_update(uint32_t teid, const in_addr_t addr) @@ -2126,89 +2145,113 @@ static void store_du_f1u_tunnel(const f1ap_drb_setup_t *drbs, int n, gNB_RRC_UE_ } } -static void fill_e1_bearer_modif_pdcp_status(gNB_RRC_UE_t *UE, - DRB_nGRAN_to_mod_t *drb_to_mod, - const int drb_id, - const bool um_on_default_drb, - const ngap_drb_status_t *drb_status) +static DRB_nGRAN_to_mod_t get_e1_drb_mod_pdcp_status(const drb_t *drb, + bearer_context_pdcp_config_t *pdcp_config, + const ngap_drb_status_t *drb_status) { DevAssert(drb_status); - drb_to_mod->id = drb_id; - drb_to_mod->pdcp_sn_status_requested = false; + DevAssert(pdcp_config); + DevAssert(drb); + DRB_nGRAN_to_mod_t drb_to_mod = {0}; + drb_to_mod.id = drb->drb_id; + drb_to_mod.pdcp_sn_status_requested = false; // PDCP SN Status Information - drb_to_mod->pdcp_config = calloc_or_fail(1, sizeof(*drb_to_mod->pdcp_config)); - drb_t *drb = get_drb(&UE->drbs, drb_id); - *drb_to_mod->pdcp_config = set_bearer_context_pdcp_config(drb->pdcp_config, um_on_default_drb, UE->redcap_cap); - drb_to_mod->pdcp_status = calloc_or_fail(1, sizeof(*drb_to_mod->pdcp_status)); - drb_to_mod->pdcp_status->dl_count.hfn = drb_status->dl_count.hfn; - drb_to_mod->pdcp_status->dl_count.sn = drb_status->dl_count.pdcp_sn; - drb_to_mod->pdcp_status->ul_count.hfn = drb_status->ul_count.hfn; - drb_to_mod->pdcp_status->ul_count.sn = drb_status->ul_count.pdcp_sn; + drb_to_mod.pdcp_config = calloc_or_fail(1, sizeof(*drb_to_mod.pdcp_config)); + *drb_to_mod.pdcp_config = *pdcp_config; + drb_to_mod.pdcp_status = calloc_or_fail(1, sizeof(*drb_to_mod.pdcp_status)); + drb_to_mod.pdcp_status->dl_count.hfn = drb_status->dl_count.hfn; + drb_to_mod.pdcp_status->dl_count.sn = drb_status->dl_count.pdcp_sn; + drb_to_mod.pdcp_status->ul_count.hfn = drb_status->ul_count.hfn; + drb_to_mod.pdcp_status->ul_count.sn = drb_status->ul_count.pdcp_sn; + return drb_to_mod; } -/** @brief Fill and send Bearer Context Modification Request with: - * (1) from F1 UE Context DRB to setup list, or (2) with PDCP Status Request */ -void e1_send_bearer_updates(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE, int n, f1ap_drb_setup_t *drbs, const ngap_drb_status_t *drb_status) +/** @brief Send E1 bearer context modification request to CU-UP */ +static void e1_send_bearer_modification_request(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE, e1ap_bearer_mod_req_t *req) { - // Quit bearer updates if no CU-UP is associated - if (!is_cuup_associated(rrc)) { + if (req->numPDUSessionsMod == 0) { + LOG_W(NR_RRC, "UE %d: No PDU sessions to modify in E1 bearer update\n", UE->rrc_ue_id); return; } - // we assume the same UE ID in CU-UP and CU-CP + DevAssert(req->numPDUSessions == 0); + req->secInfo = malloc_or_fail(sizeof(*req->secInfo)); + fill_security_info(rrc, UE, req->secInfo); + + sctp_assoc_t assoc_id = get_existing_cuup_for_ue(rrc, UE); + rrc->cucp_cuup.bearer_context_mod(assoc_id, req); +} + +/** @brief Send E1 bearer updates for DRBs to setup from F1 UE Context Modification Response */ +static void e1_send_bearer_updates(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE, int n, f1ap_drb_setup_t *drbs) +{ + if (!is_cuup_associated(rrc) || n <= 0) + return; + e1ap_bearer_mod_req_t req = { .gNB_cu_cp_ue_id = UE->rrc_ue_id, .gNB_cu_up_ue_id = UE->rrc_ue_id, }; - bool is_inter_cu_ho = UE->ho_context && UE->ho_context->source && !UE->ho_context->target; - int drb_index = 0; - FOR_EACH_SEQ_ARR(drb_t *, drb, &UE->drbs) { - int drb_id = drb->drb_id; - DRB_nGRAN_to_mod_t drb_to_mod = {0}; - if (n > 0) { - /* DRBs to Setup in F1 UE Context Modifcation Response */ - if (drb_index == n) - break; - const f1ap_drb_setup_t *drb_f1 = &drbs[drb_index]; - drb_id = drb_f1->id; - fill_e1_bearer_modif(&drb_to_mod, drb_f1); - } else if (is_inter_cu_ho) { - /* On-going handover: send PDCP Status Request */ - drb_t *drb = get_drb(&UE->drbs, drb_id); - if (!drb) { - LOG_E(NR_RRC, "Failed to send PDCP Status Request: DRB %d not found\n", drb_id); - continue; - } - drb_to_mod.id = drb_id; - // PDCP SN Status Request - drb_to_mod.pdcp_sn_status_requested = true; - LOG_I(NR_RRC, "PDCP Status requested (drb_id=%d)\n", drb_id); - } else if (drb_status) { - /* On-going handover: send PDCP Status */ - fill_e1_bearer_modif_pdcp_status(UE, &drb_to_mod, drb_id, rrc->configuration.um_on_default_drb, drb_status); - } + for (int i = 0; i < n; i++) { + const f1ap_drb_setup_t *drb_f1 = &drbs[i]; + int drb_id = drb_f1->id; rrc_pdu_session_param_t *pdu = find_pduSession_from_drbId(UE, drb_id); if (!pdu) continue; - pdu_session_to_mod_t *to_mod = find_or_next_pdu_session(&req, pdu->param.pdusession_id); - DevAssert(to_mod); - to_mod->sessionId = pdu->param.pdusession_id; - // Fill E1 DRB to Modify item - to_mod->DRBnGRanModList[to_mod->numDRB2Modify++] = drb_to_mod; - - drb_index++; + DRB_nGRAN_to_mod_t drb_to_mod = fill_e1_bearer_modif_from_f1(drb_f1); + if (!append_e1_drb_mod_req(&req, UE, drb_id, &drb_to_mod)) { + LOG_W(NR_RRC, "UE %d: Failed to append E1 DRB mod from F1 (DRB=%d)\n", UE->rrc_ue_id, drb_id); + } } - DevAssert(req.numPDUSessionsMod > 0); - DevAssert(req.numPDUSessions == 0); - // Always send security information - req.secInfo = malloc_or_fail(sizeof(*req.secInfo)); - fill_security_info(rrc, UE, req.secInfo); + e1_send_bearer_modification_request(rrc, UE, &req); + free_e1ap_context_mod_request(&req); +} - // send the E1 bearer modification request message to update F1-U tunnel info - sctp_assoc_t assoc_id = get_existing_cuup_for_ue(rrc, UE); - rrc->cucp_cuup.bearer_context_mod(assoc_id, &req); +/** @brief Request PDCP status from CU-UP during inter-CU handover */ +static void e1_request_pdcp_status(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE) +{ + if (!is_cuup_associated(rrc)) + return; + + e1ap_bearer_mod_req_t req = { + .gNB_cu_cp_ue_id = UE->rrc_ue_id, + .gNB_cu_up_ue_id = UE->rrc_ue_id, + }; + + FOR_EACH_SEQ_ARR(drb_t *, drb, &UE->drbs) { + DRB_nGRAN_to_mod_t drb_to_mod = {.id = drb->drb_id, .pdcp_sn_status_requested = true}; + LOG_D(NR_RRC, "PDCP Status requested (drb_id=%d)\n", drb->drb_id); + if (!append_e1_drb_mod_req(&req, UE, drb->drb_id, &drb_to_mod)) { + LOG_W(NR_RRC, "UE %d: Failed to append E1 DRB mod for PDCP status request (DRB=%d)\n", UE->rrc_ue_id, drb->drb_id); + } + } + + e1_send_bearer_modification_request(rrc, UE, &req); + free_e1ap_context_mod_request(&req); +} + +/** @brief Notify CU-UP with PDCP status during handover */ +void e1_notify_pdcp_status(gNB_RRC_INST *rrc, gNB_RRC_UE_t *UE, const ngap_drb_status_t *drb_status) +{ + if (!is_cuup_associated(rrc) || !drb_status) + return; + + e1ap_bearer_mod_req_t req = { + .gNB_cu_cp_ue_id = UE->rrc_ue_id, + .gNB_cu_up_ue_id = UE->rrc_ue_id, + }; + + FOR_EACH_SEQ_ARR(drb_t *, drb, &UE->drbs) { + LOG_I(NR_RRC, "Forward PDCP Status to CU-UP (drb_id=%d)\n", drb->drb_id); + bearer_context_pdcp_config_t pdcp_config = set_bearer_context_pdcp_config(drb->pdcp_config, rrc->configuration.um_on_default_drb, UE->redcap_cap); + DRB_nGRAN_to_mod_t drb_to_mod = get_e1_drb_mod_pdcp_status(drb, &pdcp_config, drb_status); + append_e1_drb_mod_req(&req, UE, drb->drb_id, &drb_to_mod); + } + + e1_send_bearer_modification_request(rrc, UE, &req); + free_e1ap_context_mod_request(&req); } static void rrc_CU_process_ue_context_setup_response(MessageDef *msg_p, instance_t instance) @@ -2262,7 +2305,7 @@ static void rrc_CU_process_ue_context_setup_response(MessageDef *msg_p, instance AssertFatal(UE->Srb[1].Active && UE->Srb[2].Active, "SRBs 1 and 2 must be active during DRB Establishment"); store_du_f1u_tunnel(resp->drbs, resp->drbs_len, UE); if (num_drb == 0) - e1_send_bearer_updates(rrc, UE, resp->drbs_len, resp->drbs, NULL); + e1_send_bearer_updates(rrc, UE, resp->drbs_len, resp->drbs); else cuup_notify_reestablishment(rrc, UE); } @@ -2397,9 +2440,11 @@ static void rrc_CU_process_ue_context_modification_response(MessageDef *msg_p, i gNB_RRC_UE_t *UE = &ue_context_p->ue_context; bool is_inter_cu_ho = UE->ho_context && UE->ho_context->source && !UE->ho_context->target; - if (resp->drbs_len > 0 || is_inter_cu_ho) { + if (resp->drbs_len > 0) { // DRB to setup store_du_f1u_tunnel(resp->drbs, resp->drbs_len, UE); - e1_send_bearer_updates(rrc, UE, resp->drbs_len, resp->drbs, NULL); + e1_send_bearer_updates(rrc, UE, resp->drbs_len, resp->drbs); + } else if (is_inter_cu_ho) { // PDCP status request + e1_request_pdcp_status(rrc, UE); } if (resp->du_to_cu_rrc_info) { diff --git a/openair2/RRC/NR/rrc_gNB_NGAP.c b/openair2/RRC/NR/rrc_gNB_NGAP.c index 8c839304c8..7d75384a9b 100644 --- a/openair2/RRC/NR/rrc_gNB_NGAP.c +++ b/openair2/RRC/NR/rrc_gNB_NGAP.c @@ -1725,7 +1725,7 @@ int rrc_gNB_process_NGAP_DL_RAN_STATUS_TRANSFER(MessageDef *msg_p, instance_t in s->dl_count.sn_len == NGAP_SN_LENGTH_18 ? "18-bit" : "12-bit"); // Send to PDCP layer - e1_send_bearer_updates(rrc, UE, 0, NULL, s); + e1_notify_pdcp_status(rrc, UE, s); } return 0; From 4aafd1e369f3ba1c95e24244c9d2ee997c66c3ee Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Thu, 25 Sep 2025 11:28:17 +0200 Subject: [PATCH 24/26] Simplify N2 handover test to focus on N2 handover only - Remove extensive traffic testing before HO (iperf tests 030001, 030002) - Do 1 ping test after attachment - Reduce ping packet count from 100 to 50 packets for remaining connectivity test Time savings: ~2-3 minutes by removing traffic validation overhead --- ci-scripts/xml_files/container_5g_rfsim_n2_ho.xml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ci-scripts/xml_files/container_5g_rfsim_n2_ho.xml b/ci-scripts/xml_files/container_5g_rfsim_n2_ho.xml index 1b2e84046c..feffaedf05 100644 --- a/ci-scripts/xml_files/container_5g_rfsim_n2_ho.xml +++ b/ci-scripts/xml_files/container_5g_rfsim_n2_ho.xml @@ -31,9 +31,6 @@ 000002 000003 020001 - 020002 - 030001 - 030002 000004 040001 050001 @@ -110,7 +107,7 @@ acamas rfsim5g_ue acamas - -c 100 -i 0.25 + -c 50 -i 0.25 5 @@ -121,7 +118,7 @@ acamas rfsim5g_ext_dn acamas - -c 100 -i 0.25 + -c 50 -i 0.25 10 From dcd7ee711bbb9029803267727db41835f8bbc0ec Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Thu, 18 Sep 2025 22:25:35 +0200 Subject: [PATCH 25/26] Improve RRC neighbor configuration documentation - Move 'What is a gNB neighbor?' explanation from rrc-usage.md to handover-tutorial.md where it belongs - Replace generic parameter names with actual configuration parameter names and specific descriptions (e.g. gNB_ID, nr_cellid) - Add example configuration structure showing proper syntax - Add missing 'band' parameter that was not listed before - Provide concrete examples and exact constraints for each parameter - Cross-reference between documents for better organization This addresses documentation issues where generic parameter descriptions were not useful for developers configuring neighbor cells. --- doc/RRC/rrc-usage.md | 55 ++++++++++++++++++++++++++-------------- doc/handover-tutorial.md | 17 +++++++++++++ 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/doc/RRC/rrc-usage.md b/doc/RRC/rrc-usage.md index 0eb7bc7195..0e8c94c61a 100644 --- a/doc/RRC/rrc-usage.md +++ b/doc/RRC/rrc-usage.md @@ -99,26 +99,43 @@ the [MAC configuration](../MAC/mac-usage.md) as well for SIB configuration. ### Neighbor-gNB configuration -#### What is a gNB neighbor? - -Network continuity is a key aspect of 5G. In the 5G architecture, gNB neighbors play a central role in maintaining service continuity through mechanisms such as handover and load balancing. By definition, a gNB neighbor is another gNB that can be measured and linked by the UE. If the current serving gNB is no longer optimal, the UE may connect to a neighbor gNB. - -To support this behavior, the network configuration specifies additional frequencies and cells that the UE should measure. The UE reports these measurements to the network, which then decides whether or not to initiate a handover. - -Neighbor types include: -- **Intra-gNB neighbors** - cells belonging to the same gNB -- **Inter-gNB neighbors** - cells belonging to different gNBs -- **Inter-RAT neighbors** - cells belonging to another RAT (e.g., LTE) +Refer to the [handover tutorial](../handover-tutorial.md) for detailed information about gNB neighbors and handover procedures. #### Required configuration parameters -To define a neighbor cell in the configuration file, the following parameters are typically needed: -- **gNB ID** - unique identifier of the gNB -- **Cell ID** - identifier of the cell within the gNB -- **Physical Cell ID** - identifier of the cell’s synchronization signal (PCI) -- **Absolute Frequency** - frequency used by the SSB (absoluteFrequencySSB) -- **Subcarrier Spacing** - numerology (e.g., 15 kHz, 30 kHz) -- **PLMN configuration** - MCC, MNC, and MNC length -- **Tracking Area Code (TAC)** - identifier of the tracking area +To define a neighbor cell in the configuration file, the following parameters are required: -Refer to the [handover tutorial](../handover-tutorial.md) for more information. +- `gNB_ID` - identifier of the neighbor gNB (e.g., `0xe01`) +- `nr_cellid` - cell identifier of the neighbor cell (e.g., `11111111`) +- `physical_cellId` - physical cell ID for radio identification (e.g., `1`) +- `absoluteFrequencySSB` - SSB frequency in ARFCN notation (e.g., `643296`) +- `subcarrierSpacing` - numerology index: 0=15kHz, 1=30kHz, 2=60kHz, 3=120kHz +- `band` - 3GPP frequency band number (e.g., `78` for 3.5GHz) +- `plmn` - PLMN configuration object with: + - `mcc` - mobile country code (3 digits, e.g., `001`) + - `mnc` - mobile network code (2-3 digits, e.g., `01`) + - `mnc_length` - number of digits in MNC (must be `2` or `3`) +- `tracking_area_code` - tracking area identifier (e.g., `1`) + +Example configuration structure: +``` +neighbour_list = ( + { + nr_cellid = 12345678; + neighbour_cell_configuration = ( + { + gNB_ID = 0xe01; + nr_cellid = 11111111; + physical_cellId = 1; + absoluteFrequencySSB = 643296; + subcarrierSpacing = 1; # 30 kHz + band = 78; + plmn = { mcc = 001; mnc = 01; mnc_length = 2 }; + tracking_area_code = 1; + } + ); + } +); +``` + +Refer to the [handover tutorial](../handover-tutorial.md) for complete examples and detailed setup instructions. diff --git a/doc/handover-tutorial.md b/doc/handover-tutorial.md index 67bf99b3c2..eeb4ce09fa 100644 --- a/doc/handover-tutorial.md +++ b/doc/handover-tutorial.md @@ -13,6 +13,23 @@ same. ![F1 Handover setup](./RRC/ho.png) +# What is a gNB neighbor? + +Network continuity is a key aspect of 5G. In the 5G architecture, gNB neighbors +play a central role in maintaining service continuity through mechanisms such +as handover and load balancing. By definition, a gNB neighbor is another gNB +that can be measured and linked by the UE. If the current serving gNB is no +longer optimal, the UE may connect to a neighbor gNB. + +To support this behavior, the network configuration specifies additional frequencies +and cells that the UE should measure. The UE reports these measurements to the +network, which then decides whether or not to initiate a handover. + +Neighbor types include: +- **Intra-gNB neighbors** - cells belonging to the same gNB +- **Inter-gNB neighbors** - cells belonging to different gNBs +- **Inter-RAT neighbors** - cells belonging to another RAT (e.g., LTE) + # Steps to run F1 handover with OAI UE Measurement reporting and processing of RRC Reconfiguration for Mobility are From dcb10ffc944c82dded1b54a8a6323c80f74e34e5 Mon Sep 17 00:00:00 2001 From: Guido Casati Date: Wed, 15 Jan 2025 10:30:00 +0100 Subject: [PATCH 26/26] Handover tutorial: fix minor issues and add same-machine setup guidance - Update opening statement to reflect both F1 (intra-gNB) and N2 (inter-gNB) handover support - Fix grammar error: 'either by by decision' -> 'either a decision' - Standardize naming: 'gNB PCI1' -> 'gNB-PCI1' - Fix typo: 'neighbor call' -> 'neighbor cell' - Remove --sa flag from N2 handover target gNB command for consistency - Add network configuration notes for same-machine setups: * IP address assignment instructions to avoid network conflicts * Target gNB command options for proper network interface configuration --- doc/handover-tutorial.md | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/doc/handover-tutorial.md b/doc/handover-tutorial.md index eeb4ce09fa..b6e60746aa 100644 --- a/doc/handover-tutorial.md +++ b/doc/handover-tutorial.md @@ -1,5 +1,5 @@ -This tutorial explains how to perform handovers. For the moment, only F1 -handovers are supported. +This tutorial explains how to perform handovers. It covers both F1 handovers +(intra-gNB, within a single gNB between DUs) and N2 handovers (inter-gNB). [[_TOC_]] @@ -370,10 +370,16 @@ We assume: * Two independent gNBs connected to the same 5GC via N2 interface. * A UE initially connected to gNB-PCI0, which will be handed over to gNB-PCI1. -* Handover is triggered by either by decision based measurement event (e.g. A3) or telnet command. +* Handover is triggered by either a decision based measurement event (e.g. A3) or telnet command. ## Steps to run N2 handover with OAI UE +**Note for same-machine setup:** When running both gNBs on the same machine, you need to assign a unique IP address to the second gNB to avoid network conflicts. For example: + +```sh +sudo ip addr add 192.168.71.180/24 dev rfsim5g-public +``` + 1. Similarly to F1 handover, UE does not support any measurement reporting and handover is triggered by telnet command. Therefore, ensure that both gNBs and UE are built with telnet support: @@ -397,10 +403,17 @@ sudo ./nr-uesoftmodem -r 106 --numerology 1 --band 78 -C 3619200000 --rfsim --ui Ensure the UE successfully registers with the network. -4. Start the target gNB (gNB PCI1) e.g. +4. Start the target gNB (gNB-PCI1) e.g. ```sh -sudo ./nr-softmodem -O ../../../targets/PROJECTS/GENERIC-NR-5GC/CONF/gnb.sa.band78.fr1.106PRB.pci1.rfsim.conf --sa --rfsim --telnetsrv --telnetsrv.shrmod ci --gNBs.[0].min_rxtxtime 6 --rfsimulator.serveraddr 127.0.0.1 +sudo ./nr-softmodem -O ../../../targets/PROJECTS/GENERIC-NR-5GC/CONF/gnb.sa.band78.fr1.106PRB.pci1.rfsim.conf --rfsim --telnetsrv --telnetsrv.shrmod ci --gNBs.[0].min_rxtxtime 6 --rfsimulator.serveraddr 127.0.0.1 +``` + +**Note for same-machine setup:** When running both gNBs on the same machine, add the following network interface options to the target gNB command, e.g.: + +```sh +--gNBs.[0].NETWORK_INTERFACES.GNB_IPV4_ADDRESS_FOR_NG_AMF 192.168.71.180 +--gNBs.[0].NETWORK_INTERFACES.GNB_IPV4_ADDRESS_FOR_NGU 192.168.71.180 ``` 5. Trigger the N2 handover, e.g. @@ -410,7 +423,7 @@ From gNB-PCI0, trigger handover on target gNB with PCI 1 for UE ID 1: ```sh echo ci trigger_n2_ho 1,1 | nc 127.0.0.1 9090 && echo ``` -where the input parameters correspond to the PCI of the neighbor call and the RRC ID of the UE. +where the input parameters correspond to the PCI of the neighbor cell and the RRC ID of the UE. This will initiate the N2 handover on the source gNB.