Fix asn1c double free

The UE stores PDCCH search spaces from the network in NR_BWP_PDCCH_t
struct within NR_UE_MAC_INST_t. After storing, the pointers otherSI_SS,
ra_SS and paging_SS are updated to point to the stored SS. Multiple
pointers could point to the same stored SS struct depending on
configuration. Then when the UE wants to release the SS, the pointers
are used to free the stored SS using ASN_STRUCT_RESET(). If multiple
pointers point to the same SS, calling ASN_STRUCT_RESET() on all
pointers would lead to double free. So one needs to check for dangling
pointer before freeing.

Instead of using pointers and assigning them during configuration, we
use SS id for otherSI_SS, ra_SS and paging_SS to retrieve the pointers
whenever needed. When the UE wants to release the SS, we make them -1
indicating an invalid SS id.

common/utils/oai_asn1.h

@@ -228,5 +228,9 @@ static inline uint64_t BIT_STRING_to_uint64(const BIT_STRING_t *asn) {
     free(STRUCT);                       \
     STRUCT = NULL;                      \
   } while (0)
+#define asn1cFreeSeq(ASN_DEF, LIST)                     \
+  do {                                                  \
+    ASN_STRUCT_FREE(ASN_DEF, LIST.array[--LIST.count]); \
+  } while (LIST.count > 0)                              \
 
 #endif

openair2/LAYER2/NR_MAC_UE/config_ue.c

@@ -43,6 +43,14 @@
 #include "oai_asn1.h"
 #include "executables/position_interface.h"
 
+#define ASIGN_P_VAL(dst, src) \
+  do {                        \
+    if (src)                  \
+      dst = *src;             \
+    else                      \
+      dst = -1;               \
+  } while (0)
+
 // Build the list of all the valid/transmitted SSBs according to the config
 static void build_ssb_list(NR_UE_MAC_INST_t *mac)
 {
@@ -497,9 +505,10 @@ static void config_common_ue(NR_UE_MAC_INST_t *mac, NR_ServingCellConfigCommon_t
 
 void release_common_ss_cset(NR_BWP_PDCCH_t *pdcch)
 {
-  asn1cFreeStruc(asn_DEF_NR_SearchSpace, pdcch->otherSI_SS);
-  asn1cFreeStruc(asn_DEF_NR_SearchSpace, pdcch->ra_SS);
-  asn1cFreeStruc(asn_DEF_NR_SearchSpace, pdcch->paging_SS);
+  pdcch->otherSI_SS_id = -1;
+  pdcch->ra_SS_id = -1;
+  pdcch->paging_SS_id = -1;
+  asn1cFreeSeq(asn_DEF_NR_SearchSpace, pdcch->list_common_SS);
   asn1cFreeStruc(asn_DEF_NR_ControlResourceSet, pdcch->commonControlResourceSet);
 }
 
@@ -521,60 +530,64 @@ static void modlist_ss(NR_SearchSpace_t *source, NR_SearchSpace_t *target)
     UPDATE_IE(target->searchSpaceType, source->searchSpaceType, struct NR_SearchSpace__searchSpaceType);
 }
 
-static NR_SearchSpace_t *get_common_search_space(const NR_UE_MAC_INST_t *mac,
-                                                 const struct NR_PDCCH_ConfigCommon__commonSearchSpaceList *commonSearchSpaceList,
-                                                 const NR_BWP_PDCCH_t *pdcch,
-                                                 const NR_SearchSpaceId_t ss_id)
+NR_SearchSpace_t *get_common_search_space(const NR_UE_MAC_INST_t *mac, const NR_SearchSpaceId_t ss_id)
 {
   if (ss_id == 0)
     return mac->search_space_zero;
 
   NR_SearchSpace_t *css = NULL;
-  for (int i = 0; i < commonSearchSpaceList->list.count; i++) {
-    if (commonSearchSpaceList->list.array[i]->searchSpaceId == ss_id) {
-      css = calloc(1, sizeof(*css));
-      modlist_ss(commonSearchSpaceList->list.array[i], css);
-      break;
+  // if current DL BWP is not set, use first BWP
+  const int bwp_id = mac->current_DL_BWP ? mac->current_DL_BWP->bwp_id : 0;
+  for (int i = 0; i < mac->config_BWP_PDCCH[bwp_id].list_common_SS.count; i++) {
+    css = mac->config_BWP_PDCCH[bwp_id].list_common_SS.array[i];
+    if (css->searchSpaceId == ss_id) {
+      return css;
     }
   }
   AssertFatal(css, "Couldn't find CSS with Id %ld\n", ss_id);
   return css;
 }
 
+static void update_ss(void *ue_ss_in, void *nw_ss_in)
+{
+  if (!nw_ss_in || !ue_ss_in)
+    return;
+
+  asn_anonymous_sequence_ *ue_ss = _A_SEQUENCE_FROM_VOID(ue_ss_in);
+  asn_anonymous_sequence_ *nw_ss = _A_SEQUENCE_FROM_VOID(nw_ss_in);
+  for (int i = 0; i < nw_ss->count; i++) {
+    NR_SearchSpace_t *source_ss = (NR_SearchSpace_t *)nw_ss->array[i];
+    NR_SearchSpace_t *target_ss = NULL;
+    for (int j = 0; j < ue_ss->count; j++) {
+      NR_SearchSpace_t **s = (NR_SearchSpace_t **)ue_ss->array;
+      if (s[j]->searchSpaceId == source_ss->searchSpaceId) {
+        target_ss = s[j];
+        break;
+      }
+    }
+    if (!target_ss) {
+      target_ss = calloc(1, sizeof(*target_ss));
+      ASN_SEQUENCE_ADD(ue_ss, target_ss);
+    }
+    modlist_ss(source_ss, target_ss);
+  }
+}
+
 static void configure_common_ss_coreset(const NR_UE_MAC_INST_t *mac,
                                         NR_BWP_PDCCH_t *pdcch,
                                         NR_PDCCH_ConfigCommon_t *pdcch_ConfigCommon)
 {
-  if (pdcch_ConfigCommon) {
-    asn1cFreeStruc(asn_DEF_NR_SearchSpace, pdcch->otherSI_SS);
-    if (pdcch_ConfigCommon->searchSpaceOtherSystemInformation)
-      pdcch->otherSI_SS = get_common_search_space(mac,
-                                                  pdcch_ConfigCommon->commonSearchSpaceList,
-                                                  pdcch,
-                                                  *pdcch_ConfigCommon->searchSpaceOtherSystemInformation);
+  if (!pdcch_ConfigCommon)
+    return;
 
-    asn1cFreeStruc(asn_DEF_NR_SearchSpace, pdcch->ra_SS);
-    if (pdcch_ConfigCommon->ra_SearchSpace) {
-      if (pdcch->otherSI_SS && *pdcch_ConfigCommon->ra_SearchSpace == pdcch->otherSI_SS->searchSpaceId)
-        pdcch->ra_SS = pdcch->otherSI_SS;
-      else
-        pdcch->ra_SS =
-            get_common_search_space(mac, pdcch_ConfigCommon->commonSearchSpaceList, pdcch, *pdcch_ConfigCommon->ra_SearchSpace);
-    }
+  if (pdcch_ConfigCommon->commonSearchSpaceList)
+    update_ss((void *)&pdcch->list_common_SS, (void *)&pdcch_ConfigCommon->commonSearchSpaceList->list);
 
-    asn1cFreeStruc(asn_DEF_NR_SearchSpace, pdcch->paging_SS);
-    if (pdcch_ConfigCommon->pagingSearchSpace) {
-      if (pdcch->otherSI_SS && *pdcch_ConfigCommon->pagingSearchSpace == pdcch->otherSI_SS->searchSpaceId)
-        pdcch->paging_SS = pdcch->otherSI_SS;
-      else if (pdcch->ra_SS && *pdcch_ConfigCommon->pagingSearchSpace == pdcch->ra_SS->searchSpaceId)
-        pdcch->paging_SS = pdcch->ra_SS;
-      if (!pdcch->paging_SS)
-        pdcch->paging_SS =
-            get_common_search_space(mac, pdcch_ConfigCommon->commonSearchSpaceList, pdcch, *pdcch_ConfigCommon->pagingSearchSpace);
-    }
+  ASIGN_P_VAL(pdcch->otherSI_SS_id, pdcch_ConfigCommon->searchSpaceOtherSystemInformation);
+  ASIGN_P_VAL(pdcch->ra_SS_id, pdcch_ConfigCommon->ra_SearchSpace);
+  ASIGN_P_VAL(pdcch->paging_SS_id, pdcch_ConfigCommon->pagingSearchSpace);
 
-    UPDATE_IE(pdcch->commonControlResourceSet, pdcch_ConfigCommon->commonControlResourceSet, NR_ControlResourceSet_t);
-  }
+  UPDATE_IE(pdcch->commonControlResourceSet, pdcch_ConfigCommon->commonControlResourceSet, NR_ControlResourceSet_t);
 }
 
 static void modlist_coreset(NR_ControlResourceSet_t *source, NR_ControlResourceSet_t *target)
@@ -672,23 +685,10 @@ static void configure_ss_coreset(NR_BWP_PDCCH_t *pdcch, NR_PDCCH_Config_t *pdcch
       }
     }
   }
-  if (pdcch_Config->searchSpacesToAddModList) {
-    for (int i = 0; i < pdcch_Config->searchSpacesToAddModList->list.count; i++) {
-      NR_SearchSpace_t *source_ss = pdcch_Config->searchSpacesToAddModList->list.array[i];
-      NR_SearchSpace_t *target_ss = NULL;
-      for (int j = 0; j < pdcch->list_SS.count; j++) {
-        if (pdcch->list_SS.array[j]->searchSpaceId == source_ss->searchSpaceId) {
-          target_ss = pdcch->list_SS.array[j];
-          break;
-        }
-      }
-      if (!target_ss) {
-        target_ss = calloc(1, sizeof(*target_ss));
-        ASN_SEQUENCE_ADD(&pdcch->list_SS, target_ss);
-      }
-      modlist_ss(source_ss, target_ss);
-    }
-  }
+
+  if (pdcch_Config->searchSpacesToAddModList)
+    update_ss((void *)&pdcch->list_SS, (void *)&pdcch_Config->searchSpacesToAddModList->list);
+
   if (pdcch_Config->searchSpacesToReleaseList) {
     for (int i = 0; i < pdcch_Config->searchSpacesToReleaseList->list.count; i++) {
       NR_ControlResourceSetId_t id = *pdcch_Config->searchSpacesToReleaseList->list.array[i];

openair2/LAYER2/NR_MAC_UE/mac_defs.h

@@ -457,11 +457,12 @@ typedef struct {
 } __attribute__ ((__packed__)) NR_CCCH_PDU;
 
 typedef struct {
-  NR_SearchSpace_t *otherSI_SS;
-  NR_SearchSpace_t *ra_SS;
-  NR_SearchSpace_t *paging_SS;
+  long otherSI_SS_id;
+  long ra_SS_id;
+  long paging_SS_id;
   NR_ControlResourceSet_t *commonControlResourceSet;
   A_SEQUENCE_OF(NR_ControlResourceSet_t) list_Coreset;
+  A_SEQUENCE_OF(NR_SearchSpace_t) list_common_SS;
   A_SEQUENCE_OF(NR_SearchSpace_t) list_SS;
 } NR_BWP_PDCCH_t;
 

openair2/LAYER2/NR_MAC_UE/mac_proto.h

@@ -417,4 +417,5 @@ void nr_mac_rrc_sl_mib_ind(const module_id_t module_id,
                            const sdu_size_t pdu_len,
                            const uint16_t rx_slss_id);
 
+NR_SearchSpace_t *get_common_search_space(const NR_UE_MAC_INST_t *mac, const NR_SearchSpaceId_t ss_id);
 #endif

openair2/LAYER2/NR_MAC_UE/nr_ue_dci_configuration.c

@@ -481,7 +481,7 @@ void ue_dci_configuration(NR_UE_MAC_INST_t *mac, fapi_nr_dl_config_request_t *dl
     // If searchSpaceOtherSystemInformation is set to zero,
     // PDCCH monitoring occasions for SI message reception in SI-window
     // are same as PDCCH monitoring occasions for SIB1
-    const NR_SearchSpace_t *ss = pdcch_config->otherSI_SS ? pdcch_config->otherSI_SS : mac->search_space_zero;
+    const NR_SearchSpace_t *ss = get_common_search_space(mac, pdcch_config->otherSI_SS_id == -1 ? 0 : pdcch_config->otherSI_SS_id);
     // TODO configure SI-window
     if (monitor_dci_for_other_SI(mac, ss, i, slots_per_frame, frame, slot)) {
       LOG_D(NR_MAC_DCI, "Monitoring DCI for other SIs in frame %d slot %d\n", frame, slot);
@@ -490,15 +490,16 @@ void ue_dci_configuration(NR_UE_MAC_INST_t *mac, fapi_nr_dl_config_request_t *dl
   }
   RA_config_t *ra = &mac->ra;
   if (mac->state == UE_PERFORMING_RA && ra->ra_state >= nrRA_WAIT_RAR) {
+    const NR_SearchSpace_t *ra_SS = get_common_search_space(mac, pdcch_config->ra_SS_id);
     // if RA is ongoing use RA search space
-    if (is_ss_monitor_occasion(frame, slot, slots_per_frame, pdcch_config->ra_SS)) {
+    if (is_ss_monitor_occasion(frame, slot, slots_per_frame, ra_SS)) {
       nr_rnti_type_t rnti_type = 0;
       if (ra->ra_type == RA_4_STEP) {
         rnti_type = ra->ra_state == nrRA_WAIT_RAR ? TYPE_RA_RNTI_ : TYPE_TC_RNTI_;
       } else {
         rnti_type = TYPE_MSGB_RNTI_;
       }
-      config_dci_pdu(mac, dl_config, rnti_type, slot, pdcch_config->ra_SS);
+      config_dci_pdu(mac, dl_config, rnti_type, slot, ra_SS);
     }
   } else if (mac->state == UE_CONNECTED) {
     for (int i = 0; i < pdcch_config->list_SS.count; i++) {
@@ -506,13 +507,14 @@ void ue_dci_configuration(NR_UE_MAC_INST_t *mac, fapi_nr_dl_config_request_t *dl
       if (is_ss_monitor_occasion(frame, slot, slots_per_frame, ss))
         config_dci_pdu(mac, dl_config, TYPE_C_RNTI_, slot, ss);
     }
-    if (pdcch_config->list_SS.count == 0 && pdcch_config->ra_SS) {
+    const NR_SearchSpace_t *ra_SS = get_common_search_space(mac, pdcch_config->ra_SS_id);
+    if (pdcch_config->list_SS.count == 0 && ra_SS) {
       // If the UE has not been provided a Type3-PDCCH CSS set or a USS set and
       // the UE has received a C-RNTI and has been provided a Type1-PDCCH CSS set,
       // the UE monitors PDCCH candidates for DCI format 0_0 and DCI format 1_0
       // with CRC scrambled by the C-RNTI in the Type1-PDCCH CSS set
-      if (is_ss_monitor_occasion(frame, slot, slots_per_frame, pdcch_config->ra_SS))
-        config_dci_pdu(mac, dl_config, TYPE_C_RNTI_, slot, pdcch_config->ra_SS);
+      if (is_ss_monitor_occasion(frame, slot, slots_per_frame, ra_SS))
+        config_dci_pdu(mac, dl_config, TYPE_C_RNTI_, slot, ra_SS);
     }
   }
 }

openair2/LAYER2/NR_MAC_UE/nr_ue_procedures.c

@@ -4002,9 +4002,10 @@ static void handle_rar_reception(NR_UE_MAC_INST_t *mac, NR_MAC_RAR *rar, frame_t
   const NR_UE_UL_BWP_t *current_UL_BWP = mac->current_UL_BWP;
   const NR_UE_DL_BWP_t *current_DL_BWP = mac->current_DL_BWP;
   const NR_BWP_PDCCH_t *pdcch_config = &mac->config_BWP_PDCCH[current_DL_BWP->bwp_id];
+  const NR_SearchSpace_t *ra_SS = get_common_search_space(mac, pdcch_config->ra_SS_id);
   NR_tda_info_t tda_info = get_ul_tda_info(current_UL_BWP,
-                                           *pdcch_config->ra_SS->controlResourceSetId,
-                                           pdcch_config->ra_SS->searchSpaceType->present,
+                                           *ra_SS->controlResourceSetId,
+                                           ra_SS->searchSpaceType->present,
                                            TYPE_RA_RNTI_,
                                            rar_grant.Msg3_t_alloc);
   if (!tda_info.valid_tda || tda_info.nrOfSymbols == 0) {

openair2/LAYER2/NR_MAC_UE/nr_ue_scheduler.c

@@ -2197,8 +2197,9 @@ static void nr_ue_prach_scheduler(NR_UE_MAC_INST_t *mac, frame_t frameP, slot_t
         int next_frame = (frameP + (next_slot < slotP)) % MAX_FRAME_NUMBER;
         int add_slots = 1;
         NR_BWP_PDCCH_t *pdcch_config = &mac->config_BWP_PDCCH[mac->current_DL_BWP->bwp_id];
+        const NR_SearchSpace_t *ra_SS = get_common_search_space(mac, pdcch_config->ra_SS_id);
         while (!is_dl_slot(next_slot, &mac->frame_structure)
-               || !is_ss_monitor_occasion(next_frame, next_slot, n_slots_frame, pdcch_config->ra_SS)) {
+               || !is_ss_monitor_occasion(next_frame, next_slot, n_slots_frame, ra_SS)) {
           int temp_slot = (next_slot + 1) % n_slots_frame;
           next_frame = (next_frame + (temp_slot < next_slot)) % MAX_FRAME_NUMBER;
           next_slot = temp_slot;