fix handling of invalid MAC LCIDs

lib/include/srslte/common/mac_nr_pdu.h

@@ -60,10 +60,11 @@ public:
 
   nr_lcid_sch_t get_type();
   bool          is_sdu();
+  bool          is_valid_lcid();
   bool          is_var_len_ce();
   bool          is_ul_ccch();
 
-  uint32_t read_subheader(const uint8_t* ptr);
+  int32_t  read_subheader(const uint8_t* ptr);
   uint32_t get_total_length();
   uint32_t get_sdu_length();
   uint32_t get_lcid();

lib/src/common/mac_nr_pdu.cc

@@ -36,7 +36,14 @@ mac_nr_sch_subpdu::nr_lcid_sch_t mac_nr_sch_subpdu::get_type()
 
 bool mac_nr_sch_subpdu::is_sdu()
 {
-  return get_type() == CCCH;
+  // for UL-SCH LCID 52 is also valid for carrying SDUs
+  return (lcid <= 32 || (parent->is_ulsch() && lcid == 52));
+}
+
+// returns false for all reserved values in Table 6.2.1-1 and 6.2.1-2
+bool mac_nr_sch_subpdu::is_valid_lcid()
+{
+  return (lcid <= 63 && ((parent->is_ulsch() && (lcid <= 32 || lcid >= 52)) || (lcid <= 32 || lcid >= 47)));
 }
 
 bool mac_nr_sch_subpdu::is_var_len_ce()
@@ -44,8 +51,8 @@ bool mac_nr_sch_subpdu::is_var_len_ce()
   return false;
 }
 
-// return length of PDU
+// return length of PDU (or SRSLTE_ERROR otherwise)
-uint32_t mac_nr_sch_subpdu::read_subheader(const uint8_t* ptr)
+int32_t mac_nr_sch_subpdu::read_subheader(const uint8_t* ptr)
 {
   // Skip R, read F bit and LCID
   F_bit = (bool)(*ptr & 0x40) ? true : false;
@@ -53,6 +60,7 @@ uint32_t mac_nr_sch_subpdu::read_subheader(const uint8_t* ptr)
   ptr++;
   header_length = 1;
 
+  if (is_valid_lcid()) {
     if ((is_sdu() || is_var_len_ce()) && not is_ul_ccch()) {
       // Read first length byte
       sdu_length = (uint32_t)*ptr;
@@ -69,6 +77,10 @@ uint32_t mac_nr_sch_subpdu::read_subheader(const uint8_t* ptr)
       sdu_length = sizeof_ce(lcid, parent->is_ulsch());
     }
     sdu = (uint8_t*)ptr;
+  } else {
+    fprintf(stderr, "Invalid LCID (%d) in MAC PDU\n", lcid);
+    return SRSLTE_ERROR;
+  }
   return header_length;
 }
 
@@ -215,7 +227,10 @@ void mac_nr_sch_pdu::unpack(const uint8_t* payload, const uint32_t& len)
   uint32_t offset = 0;
   while (offset < len) {
     mac_nr_sch_subpdu sch_pdu(this);
-    sch_pdu.read_subheader(payload + offset);
+    if (sch_pdu.read_subheader(payload + offset) == SRSLTE_ERROR) {
+      fprintf(stderr, "Error parsing NR MAC PDU (len=%d, offset=%d)\n", len, offset);
+      return;
+    }
     offset += sch_pdu.get_total_length();
     subpdus.push_back(sch_pdu);
   }

lib/test/common/mac_nr_pdu_test.cc

@@ -247,6 +247,28 @@ int mac_dl_sch_pdu_pack_test5()
   return SRSLTE_SUCCESS;
 }
 
+int mac_dl_sch_pdu_unpack_test6()
+{
+  // MAC PDU with DL-SCH subheader reserved LCID
+  // Bit 1-8
+  // |   |   |   |   |   |   |   |   |
+  // | R |F=1|         LCID          |  Octet 1
+  // |               L               |  Octet 2
+
+  // TV2 - MAC PDU with reserved LCID (46=0x2e)
+  uint8_t mac_dl_sch_pdu_2[] = {0x2e, 0x04, 0x11, 0x22, 0x33, 0x44};
+
+  if (pcap_handle) {
+    pcap_handle->write_dl_crnti(mac_dl_sch_pdu_2, sizeof(mac_dl_sch_pdu_2), PCAP_CRNTI, true, PCAP_TTI);
+  }
+
+  srslte::mac_nr_sch_pdu pdu;
+  pdu.unpack(mac_dl_sch_pdu_2, sizeof(mac_dl_sch_pdu_2));
+  TESTASSERT(pdu.get_num_subpdus() == 0);
+
+  return SRSLTE_SUCCESS;
+}
+
 int mac_ul_sch_pdu_unpack_test1()
 {
   // UL-SCH MAC PDU with fixed-size CE and DL-SCH subheader with 16-bit length field
@@ -337,6 +359,117 @@ int mac_ul_sch_pdu_unpack_and_pack_test2()
   return SRSLTE_SUCCESS;
 }
 
+int mac_ul_sch_pdu_unpack_and_pack_test3()
+{
+  // MAC PDU with UL-SCH (with normal LCID) subheader for short SDU
+  // Bit 1-8
+  // |   |   |   |   |   |   |   |   |
+  // | R |F=0|         LCID          |  Octet 1
+  // |                L              |  Octet 2
+
+  // TV1 - MAC PDU with short subheader for CCCH, MAC SDU length is 8 B, total PDU is 10 B
+  uint8_t mac_ul_sch_pdu_1[] = {0x02, 0x0a, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa};
+
+  if (pcap_handle) {
+    pcap_handle->write_ul_crnti(mac_ul_sch_pdu_1, sizeof(mac_ul_sch_pdu_1), PCAP_CRNTI, true, PCAP_TTI);
+  }
+
+  srslte::mac_nr_sch_pdu pdu(true);
+  pdu.unpack(mac_ul_sch_pdu_1, sizeof(mac_ul_sch_pdu_1));
+  TESTASSERT(pdu.get_num_subpdus() == 1);
+
+  mac_nr_sch_subpdu subpdu = pdu.get_subpdu(0);
+  TESTASSERT(subpdu.get_total_length() == 12);
+  TESTASSERT(subpdu.get_sdu_length() == 10);
+  TESTASSERT(subpdu.get_lcid() == 2);
+
+  // pack PDU again
+  byte_buffer_t tx_buffer;
+
+  srslte::mac_nr_sch_pdu tx_pdu;
+  tx_pdu.init_tx(&tx_buffer, sizeof(mac_ul_sch_pdu_1), true);
+
+  // Add SDU part of TV from above
+  tx_pdu.add_sdu(2, &mac_ul_sch_pdu_1[2], 10);
+
+  TESTASSERT(tx_pdu.get_remaing_len() == 0);
+  TESTASSERT(tx_buffer.N_bytes == sizeof(mac_ul_sch_pdu_1));
+  TESTASSERT(memcmp(tx_buffer.msg, mac_ul_sch_pdu_1, tx_buffer.N_bytes) == 0);
+
+  if (pcap_handle) {
+    pcap_handle->write_ul_crnti(tx_buffer.msg, tx_buffer.N_bytes, PCAP_CRNTI, true, PCAP_TTI);
+  }
+
+  srslte::log_filter log("MAC");
+  log.set_level(srslte::LOG_LEVEL_DEBUG);
+  log.set_hex_limit(100000);
+  log.info_hex(tx_buffer.msg, tx_buffer.N_bytes, "Generated MAC PDU (%d B)\n", tx_buffer.N_bytes);
+
+  return SRSLTE_SUCCESS;
+}
+
+int mac_ul_sch_pdu_pack_test4()
+{
+  // MAC PDU with UL-SCH (with normal LCID) subheader for long SDU
+  // Bit 1-8
+  // |   |   |   |   |   |   |   |   |
+  // | R |F=1|         LCID          |  Octet 1
+  // |                L              |  Octet 2
+  // |                L              |  Octet 3
+
+  uint8_t sdu[512] = {};
+
+  // populate SDU payload
+  for (uint32_t i = 0; i < 512; i++) {
+    sdu[i] = i % 256;
+  }
+
+  // pack PDU again
+  byte_buffer_t tx_buffer;
+
+  srslte::mac_nr_sch_pdu tx_pdu;
+  tx_pdu.init_tx(&tx_buffer, sizeof(sdu) + 3, true);
+
+  // Add SDU part of TV from above
+  tx_pdu.add_sdu(2, sdu, sizeof(sdu));
+
+  TESTASSERT(tx_pdu.get_remaing_len() == 0);
+  TESTASSERT(tx_buffer.N_bytes == sizeof(sdu) + 3);
+
+  if (pcap_handle) {
+    pcap_handle->write_ul_crnti(tx_buffer.msg, tx_buffer.N_bytes, PCAP_CRNTI, true, PCAP_TTI);
+  }
+
+  srslte::log_filter log("MAC");
+  log.set_level(srslte::LOG_LEVEL_DEBUG);
+  log.set_hex_limit(100000);
+  log.info_hex(tx_buffer.msg, tx_buffer.N_bytes, "Generated MAC PDU (%d B)\n", tx_buffer.N_bytes);
+
+  return SRSLTE_SUCCESS;
+}
+
+int mac_ul_sch_pdu_unpack_test5()
+{
+  // MAC PDU with UL-SCH (with normal LCID) subheader for short SDU but reserved LCID
+  // Bit 1-8
+  // |   |   |   |   |   |   |   |   |
+  // | R |F=0|         LCID          |  Octet 1
+  // |                L              |  Octet 2
+
+  // TV1 - MAC PDU with short subheader but reserved LCID for UL-SCH (LCID=33)
+  uint8_t mac_ul_sch_pdu_1[] = {0x21, 0x0a, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa};
+
+  if (pcap_handle) {
+    pcap_handle->write_ul_crnti(mac_ul_sch_pdu_1, sizeof(mac_ul_sch_pdu_1), PCAP_CRNTI, true, PCAP_TTI);
+  }
+
+  srslte::mac_nr_sch_pdu pdu(true);
+  pdu.unpack(mac_ul_sch_pdu_1, sizeof(mac_ul_sch_pdu_1));
+  TESTASSERT(pdu.get_num_subpdus() == 0);
+
+  return SRSLTE_SUCCESS;
+}
+
 int main(int argc, char** argv)
 {
 #if PCAP
@@ -369,6 +502,11 @@ int main(int argc, char** argv)
     return SRSLTE_ERROR;
   }
 
+  if (mac_dl_sch_pdu_unpack_test6()) {
+    fprintf(stderr, "mac_dl_sch_pdu_unpack_test6() failed.\n");
+    return SRSLTE_ERROR;
+  }
+
   if (mac_ul_sch_pdu_unpack_test1()) {
     fprintf(stderr, "mac_ul_sch_pdu_unpack_test1() failed.\n");
     return SRSLTE_ERROR;
@@ -379,5 +517,20 @@ int main(int argc, char** argv)
     return SRSLTE_ERROR;
   }
 
+  if (mac_ul_sch_pdu_unpack_and_pack_test3()) {
+    fprintf(stderr, "mac_ul_sch_pdu_unpack_and_pack_test3() failed.\n");
+    return SRSLTE_ERROR;
+  }
+
+  if (mac_ul_sch_pdu_pack_test4()) {
+    fprintf(stderr, "mac_ul_sch_pdu_pack_test4() failed.\n");
+    return SRSLTE_ERROR;
+  }
+
+  if (mac_ul_sch_pdu_unpack_test5()) {
+    fprintf(stderr, "mac_ul_sch_pdu_unpack_test5() failed.\n");
+    return SRSLTE_ERROR;
+  }
+
   return SRSLTE_SUCCESS;
 }