ue,nr,mac: fix race-condition when accesing temp RNTIs during RA procedure

the race was in the prach_nr that stored the temp crnti without proper protection.
the fix moves the logic to store the value to the MAC that uses the thread-safe
RNTI object for this.

srsue/hdr/stack/mac_nr/mac_nr.h

@@ -92,6 +92,8 @@ public:
   uint16_t get_crnti();
   uint16_t get_temp_crnti();
   uint16_t get_csrnti() { return SRSRAN_INVALID_RNTI; }; // SPS not supported
+  void     set_temp_crnti(uint16_t temp_crnti);
+  void     set_crnti_to_temp();
 
   /// procedure sr nr interface
   void start_ra() { proc_ra.start_by_mac(); }
@@ -130,6 +132,7 @@ private:
   bool is_paging_opportunity();
 
   bool has_crnti();
+  bool has_temp_crnti();
   bool is_valid_crnti(const uint16_t crnti);
 
   std::vector<srsran::logical_channel_config_t> logical_channels; // stores the raw configs provide by upper layers

srsue/hdr/stack/mac_nr/mac_nr_interfaces.h

@@ -26,6 +26,8 @@ public:
   // Functions for identity handling, e.g., contention id and c-rnti
   virtual uint16_t get_crnti()                = 0;
   virtual bool     set_crnti(uint16_t c_rnti) = 0;
+  virtual void     set_temp_crnti(uint16_t c_rnti) = 0;
+  virtual void     set_crnti_to_temp()             = 0;
 
   // Functions for msg3 manipulation which shall be transparent to the procedure
   virtual bool msg3_is_transmitted() = 0;

srsue/hdr/stack/mac_nr/proc_ra_nr.h

@@ -40,6 +40,7 @@ public:
   uint16_t get_rar_rnti();
   bool     has_temp_crnti();
   uint16_t get_temp_crnti();
+  void     set_crnti_to_temp();
   void     received_contention_resolution(bool is_successful);
 
   // PHY interfaces
@@ -64,8 +65,6 @@ private:
 
   int        ra_window_length = -1, ra_window_start = -1;
   uint16_t   rar_rnti   = SRSRAN_INVALID_RNTI;
-  uint16_t   temp_crnti = SRSRAN_INVALID_RNTI;
-  uint16_t   transmitted_crnti = SRSRAN_INVALID_RNTI;
   std::mutex mutex;
 
   srsran::rach_cfg_nr_t rach_cfg   = {};

srsue/src/stack/mac_nr/mac_nr.cc

@@ -163,9 +163,9 @@ void mac_nr::update_buffer_states()
 
 mac_interface_phy_nr::sched_rnti_t mac_nr::get_ul_sched_rnti_nr(const uint32_t tti)
 {
-  if (proc_ra.has_temp_crnti() && has_crnti() == false) {
+  if (has_temp_crnti() && has_crnti() == false) {
-    logger.debug("SCHED: Searching temp C-RNTI=0x%x (proc_ra)", proc_ra.get_temp_crnti());
+    logger.debug("SCHED: Searching temp C-RNTI=0x%x (proc_ra)", rntis.get_temp_rnti());
-    return {proc_ra.get_temp_crnti(), srsran_rnti_type_c};
+    return {rntis.get_temp_rnti(), srsran_rnti_type_c};
   }
   return {rntis.get_crnti(), srsran_rnti_type_c};
 }
@@ -195,9 +195,9 @@ mac_interface_phy_nr::sched_rnti_t mac_nr::get_dl_sched_rnti_nr(const uint32_t t
     return {proc_ra.get_rar_rnti(), srsran_rnti_type_ra};
   }
 
-  if (proc_ra.has_temp_crnti() && has_crnti() == false) {
+  if (has_temp_crnti() && has_crnti() == false) {
-    logger.debug("SCHED: Searching temp C-RNTI=0x%x (proc_ra)", proc_ra.get_temp_crnti());
+    logger.debug("SCHED: Searching temp C-RNTI=0x%x (proc_ra)", rntis.get_temp_rnti());
-    return {proc_ra.get_temp_crnti(), srsran_rnti_type_c};
+    return {rntis.get_temp_rnti(), srsran_rnti_type_c};
   }
 
   if (has_crnti()) {
@@ -209,6 +209,26 @@ mac_interface_phy_nr::sched_rnti_t mac_nr::get_dl_sched_rnti_nr(const uint32_t t
   return {SRSRAN_INVALID_RNTI, srsran_rnti_type_c};
 }
 
+bool mac_nr::has_temp_crnti()
+{
+  return rntis.get_temp_rnti() != SRSRAN_INVALID_RNTI;
+}
+
+uint16_t mac_nr::get_temp_crnti()
+{
+  return rntis.get_temp_rnti();
+}
+
+void mac_nr::set_temp_crnti(uint16_t temp_crnti)
+{
+  rntis.set_temp_rnti(temp_crnti);
+}
+
+void mac_nr::set_crnti_to_temp()
+{
+  rntis.set_crnti_to_temp();
+}
+
 bool mac_nr::has_crnti()
 {
   return rntis.get_crnti() != SRSRAN_INVALID_RNTI;
@@ -219,11 +239,6 @@ uint16_t mac_nr::get_crnti()
   return rntis.get_crnti();
 }
 
-uint16_t mac_nr::get_temp_crnti()
-{
-  return proc_ra.get_temp_crnti();
-}
-
 srsran::mac_sch_subpdu_nr::lcg_bsr_t mac_nr::generate_sbsr()
 {
   return proc_bsr.generate_sbsr();
@@ -309,7 +324,7 @@ void mac_nr::new_grant_dl(const uint32_t cc_idx, const mac_nr_grant_dl_t& grant,
 
 void mac_nr::tb_decoded(const uint32_t cc_idx, const mac_nr_grant_dl_t& grant, tb_action_dl_result_t result)
 {
-  logger.debug("tb_decoded(): cc_idx=%d, tti=%d, rnti=%d, pid=%d, tbs=%d, ndi=%d, rv=%d, result=%s",
+  logger.debug("tb_decoded(): cc_idx=%d, tti=%d, rnti=0x%X, pid=%d, tbs=%d, ndi=%d, rv=%d, result=%s",
                cc_idx,
                grant.tti,
                grant.rnti,
@@ -336,14 +351,14 @@ void mac_nr::tb_decoded(const uint32_t cc_idx, const mac_nr_grant_dl_t& grant, t
   }
 
   // If proc ra is in contention resolution (RA connection request procedure)
-  if (proc_ra.is_contention_resolution() && grant.rnti == get_temp_crnti()) {
+  if (proc_ra.is_contention_resolution() && grant.rnti == rntis.get_temp_rnti()) {
     proc_ra.received_contention_resolution(contention_res_successful);
   }
 }
 
 void mac_nr::new_grant_ul(const uint32_t cc_idx, const mac_nr_grant_ul_t& grant, tb_action_ul_t* action)
 {
-  logger.debug("new_grant_ul(): cc_idx=%d, tti=%d, rnti=%d, pid=%d, tbs=%d, ndi=%d, rv=%d, is_rar=%d",
+  logger.debug("new_grant_ul(): cc_idx=%d, tti=%d, rnti=0x%X, pid=%d, tbs=%d, ndi=%d, rv=%d, is_rar=%d",
                cc_idx,
                grant.tti,
                grant.rnti,

srsue/src/stack/mac_nr/proc_ra_nr.cc

@@ -115,18 +115,6 @@ bool proc_ra_nr::has_rar_rnti()
   return false;
 }
 
-bool proc_ra_nr::has_temp_crnti()
-{
-  std::lock_guard<std::mutex> lock(mutex);
-  return temp_crnti != SRSRAN_INVALID_RNTI;
-}
-
-uint16_t proc_ra_nr::get_temp_crnti()
-{
-  std::lock_guard<std::mutex> lock(mutex);
-  return temp_crnti;
-}
-
 void proc_ra_nr::received_contention_resolution(bool is_successful)
 {
   std::lock_guard<std::mutex> lock(mutex);
@@ -216,13 +204,10 @@ void proc_ra_nr::ra_response_reception(const mac_interface_phy_nr::tb_action_dl_
     for (auto& subpdu : pdu.get_subpdus()) {
       if (subpdu.has_rapid() && subpdu.get_rapid() == preamble_index) {
         logger.debug("PROC RA NR: Setting UL grant and prepare Msg3");
-        temp_crnti = subpdu.get_temp_crnti();
+        mac.set_temp_crnti(subpdu.get_temp_crnti());
-
-        // Save transmitted C-RNTI (if any)
-        transmitted_crnti = mac.get_crnti();
 
         // Set Temporary-C-RNTI if provided, otherwise C-RNTI is ok
-        phy->set_rar_grant(tb.rx_slot_idx, subpdu.get_ul_grant(), temp_crnti, srsran_rnti_type_ra);
+        phy->set_rar_grant(tb.rx_slot_idx, subpdu.get_ul_grant(), subpdu.get_temp_crnti(), srsran_rnti_type_ra);
 
         // Apply TA CMD
         current_ta = subpdu.get_ta();
@@ -280,12 +265,17 @@ void proc_ra_nr::ra_completion()
                    srsran::enum_to_text(state_str_nr, (uint32_t)ra_state_t::MAX_RA_STATES, WAITING_FOR_COMPLETION));
     return;
   }
+
+  // Start looking for PDCCH CRNTI
+  if (!mac.get_crnti()) {
+    // promote temp RNTI to new C-RNTI
+    mac.set_crnti_to_temp();
+    mac.set_temp_crnti(SRSRAN_INVALID_RNTI);
+  }
+
   srsran::console("Random Access Complete.     c-rnti=0x%x, ta=%d\n", mac.get_crnti(), current_ta);
   logger.info("Random Access Complete.     c-rnti=0x%x, ta=%d", mac.get_crnti(), current_ta);
-  if (!transmitted_crnti) {
+
-    mac.set_crnti(temp_crnti);
-  }
-  temp_crnti = SRSRAN_INVALID_RNTI;
   mac.rrc_ra_completed();
   reset();
 }
@@ -293,9 +283,9 @@ void proc_ra_nr::ra_completion()
 void proc_ra_nr::ra_error()
 {
   std::lock_guard<std::mutex> lock(mutex);
-  temp_crnti = SRSRAN_INVALID_RNTI;
   preamble_transmission_counter++;
   contention_resolution_timer.stop();
+  mac.set_temp_crnti(SRSRAN_INVALID_RNTI);
   uint32_t backoff_wait;
   bool     ra_procedure_completed = false; // true = (unsuccessfully) completed, false = uncompleted
 
@@ -389,10 +379,10 @@ void proc_ra_nr::reset()
 {
   state      = IDLE;
   started_by = initiators_t::initiators_t_NULLTYPE;
+  mac.set_temp_crnti(SRSRAN_INVALID_RNTI);
   prach_send_timer.stop();
   rar_timeout_timer.stop();
   contention_resolution_timer.stop();
-  transmitted_crnti = SRSRAN_INVALID_RNTI;
 }
 
 } // namespace srsue

srsue/src/stack/mac_nr/test/proc_ra_nr_test.cc

@@ -66,6 +66,8 @@ public:
     crnti = c_rnti;
     return true;
   }
+  void set_temp_crnti(uint16_t c_rnti) {}
+  void set_crnti_to_temp() {}
 
   bool msg3_is_transmitted() { return true; }
   void msg3_flush() {}