From 885a029abcc25b29b0c28243d9c01eb9e318cc83 Mon Sep 17 00:00:00 2001
From: Robert Schmidt <robert.schmidt@openairinterface.org>
Date: Mon, 21 Aug 2023 14:00:46 +0200
Subject: [PATCH] Fix memory leaks in do_NR_DLInformationTransfer()

---
 openair2/RRC/NR/MESSAGES/asn1_msg.c | 55 +++++++++++++----------------
 openair2/RRC/NR/MESSAGES/asn1_msg.h | 14 ++++----
 openair2/RRC/NR/rrc_gNB_NGAP.c      |  9 +++--
 3 files changed, 38 insertions(+), 40 deletions(-)

diff --git a/openair2/RRC/NR/MESSAGES/asn1_msg.c b/openair2/RRC/NR/MESSAGES/asn1_msg.c
index 35ca0115e6..d7dbe0dd1d 100644
--- a/openair2/RRC/NR/MESSAGES/asn1_msg.c
+++ b/openair2/RRC/NR/MESSAGES/asn1_msg.c
@@ -948,41 +948,36 @@ uint8_t do_RRCSetupComplete(uint8_t Mod_id, uint8_t *buffer, size_t buffer_size,
 }
 
 //------------------------------------------------------------------------------
-uint8_t 
-do_NR_DLInformationTransfer(
-    uint8_t Mod_id,
-    uint8_t **buffer,
-    uint8_t transaction_id,
-    uint32_t pdu_length,
-    uint8_t *pdu_buffer
-)
+uint8_t do_NR_DLInformationTransfer(uint8_t Mod_id,
+                                    uint8_t *buffer,
+                                    size_t buffer_len,
+                                    uint8_t transaction_id,
+                                    uint32_t pdu_length,
+                                    uint8_t *pdu_buffer)
 //------------------------------------------------------------------------------
 {
-    ssize_t encoded;
-    NR_DL_DCCH_Message_t   dl_dcch_msg={0};
-    dl_dcch_msg.message.present            = NR_DL_DCCH_MessageType_PR_c1;
-    asn1cCalloc(dl_dcch_msg.message.choice.c1, c1);
-    c1->present = NR_DL_DCCH_MessageType__c1_PR_dlInformationTransfer;
+  NR_DL_DCCH_Message_t dl_dcch_msg = {0};
+  dl_dcch_msg.message.present = NR_DL_DCCH_MessageType_PR_c1;
+  asn1cCalloc(dl_dcch_msg.message.choice.c1, c1);
+  c1->present = NR_DL_DCCH_MessageType__c1_PR_dlInformationTransfer;
 
-    asn1cCalloc(c1->choice.dlInformationTransfer, infoTransfer);
-    infoTransfer->rrc_TransactionIdentifier = transaction_id;
-    infoTransfer->criticalExtensions.present =
-        NR_DLInformationTransfer__criticalExtensions_PR_dlInformationTransfer;
+  asn1cCalloc(c1->choice.dlInformationTransfer, infoTransfer);
+  infoTransfer->rrc_TransactionIdentifier = transaction_id;
+  infoTransfer->criticalExtensions.present = NR_DLInformationTransfer__criticalExtensions_PR_dlInformationTransfer;
 
-    asn1cCalloc(infoTransfer->criticalExtensions.choice.dlInformationTransfer, dlInfoTransfer);
-    asn1cCalloc(dlInfoTransfer->dedicatedNAS_Message,msg);
-    // we will free the caller buffer, that is ok in the present code logic (else it will leak memory) but not natural,
-    // comprehensive code design
-    msg->buf = pdu_buffer;
-    msg->size = pdu_length;
+  asn1cCalloc(infoTransfer->criticalExtensions.choice.dlInformationTransfer, dlInfoTransfer);
+  asn1cCalloc(dlInfoTransfer->dedicatedNAS_Message, msg);
+  // we will free the caller buffer, that is ok in the present code logic (else it will leak memory) but not natural,
+  // comprehensive code design
+  msg->buf = pdu_buffer;
+  msg->size = pdu_length;
 
-    encoded = uper_encode_to_new_buffer (&asn_DEF_NR_DL_DCCH_Message, NULL, (void *) &dl_dcch_msg, (void **)buffer);
-    AssertFatal(encoded > 0,"ASN1 message encoding failed (%s, %ld)!\n",
-                "DLInformationTransfer", encoded);
-    ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NR_DL_DCCH_Message,&dl_dcch_msg );
-    LOG_D(NR_RRC,"DLInformationTransfer Encoded %zd bytes\n", encoded);
-    //for (int i=0;i<encoded;i++) printf("%02x ",(*buffer)[i]);
-    return encoded;
+  asn_enc_rval_t r = uper_encode_to_buffer(&asn_DEF_NR_DL_DCCH_Message, NULL, (void *)&dl_dcch_msg, buffer, buffer_len);
+  AssertFatal(r.encoded > 0, "ASN1 message encoding failed (%s, %ld)!\n", "DLInformationTransfer", r.encoded);
+  ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_NR_DL_DCCH_Message, &dl_dcch_msg);
+  LOG_D(NR_RRC, "DLInformationTransfer Encoded %zd bytes\n", r.encoded);
+  // for (int i=0;i<encoded;i++) printf("%02x ",(*buffer)[i]);
+  return r.encoded;
 }
 
 uint8_t do_NR_ULInformationTransfer(uint8_t **buffer, uint32_t pdu_length, uint8_t *pdu_buffer) {
diff --git a/openair2/RRC/NR/MESSAGES/asn1_msg.h b/openair2/RRC/NR/MESSAGES/asn1_msg.h
index 83c4e4897c..7971c79533 100644
--- a/openair2/RRC/NR/MESSAGES/asn1_msg.h
+++ b/openair2/RRC/NR/MESSAGES/asn1_msg.h
@@ -128,14 +128,12 @@ uint8_t do_NR_RRCReconfigurationComplete(
                         const uint8_t Transaction_id
                       );
 
-uint8_t 
-do_NR_DLInformationTransfer(
-    uint8_t Mod_id,
-    uint8_t **buffer,
-    uint8_t transaction_id,
-    uint32_t pdu_length,
-    uint8_t *pdu_buffer
-);
+uint8_t do_NR_DLInformationTransfer(uint8_t Mod_id,
+                                    uint8_t *buffer,
+                                    size_t buffer_len,
+                                    uint8_t transaction_id,
+                                    uint32_t pdu_length,
+                                    uint8_t *pdu_buffer);
 
 uint8_t do_NR_ULInformationTransfer(uint8_t **buffer, 
                         uint32_t pdu_length,
diff --git a/openair2/RRC/NR/rrc_gNB_NGAP.c b/openair2/RRC/NR/rrc_gNB_NGAP.c
index d2313761dd..d75fe69fe0 100644
--- a/openair2/RRC/NR/rrc_gNB_NGAP.c
+++ b/openair2/RRC/NR/rrc_gNB_NGAP.c
@@ -588,7 +588,7 @@ int rrc_gNB_process_NGAP_DOWNLINK_NAS(MessageDef *msg_p, instance_t instance, mu
 //------------------------------------------------------------------------------
 {
   uint32_t length;
-  uint8_t *buffer;
+  uint8_t buffer[4096];
   protocol_ctxt_t ctxt = {0};
   ngap_downlink_nas_t *req = &NGAP_DOWNLINK_NAS(msg_p);
   rrc_gNB_ue_context_t *ue_context_p = rrc_gNB_get_ue_context(RC.nrrrc[instance], req->gNB_ue_ngap_id);
@@ -611,7 +611,12 @@ int rrc_gNB_process_NGAP_DOWNLINK_NAS(MessageDef *msg_p, instance_t instance, mu
   PROTOCOL_CTXT_SET_BY_INSTANCE(&ctxt, instance, GNB_FLAG_YES, UE->rrc_ue_id, 0, 0);
 
   /* Create message for PDCP (DLInformationTransfer_t) */
-  length = do_NR_DLInformationTransfer(instance, &buffer, rrc_gNB_get_next_transaction_identifier(instance), req->nas_pdu.length, req->nas_pdu.buffer);
+  length = do_NR_DLInformationTransfer(instance,
+                                       buffer,
+                                       sizeof(buffer),
+                                       rrc_gNB_get_next_transaction_identifier(instance),
+                                       req->nas_pdu.length,
+                                       req->nas_pdu.buffer);
   LOG_DUMPMSG(NR_RRC, DEBUG_RRC, buffer, length, "[MSG] RRC DL Information Transfer\n");
   /*
    * switch UL or DL NAS message without RRC piggybacked to SRB2 if active.