From 04e6828c62cd626505a54414799e870882dc82cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Soares=20Silva?= Date: Mon, 1 Jul 2024 10:36:30 +0100 Subject: [PATCH 1/4] Change functions using RX_DATA.indication PDU_Length to expect 32 bits --- nfapi/open-nFAPI/nfapi/src/nfapi_p7.c | 41 +++++++++---------- .../LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c | 4 +- openair2/LAYER2/NR_MAC_gNB/mac_proto.h | 2 +- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/nfapi/open-nFAPI/nfapi/src/nfapi_p7.c b/nfapi/open-nFAPI/nfapi/src/nfapi_p7.c index b856e6b648..9622cd7f5e 100644 --- a/nfapi/open-nFAPI/nfapi/src/nfapi_p7.c +++ b/nfapi/open-nFAPI/nfapi/src/nfapi_p7.c @@ -3177,18 +3177,16 @@ uint8_t pack_nr_slot_indication(void *msg, uint8_t **ppWritePackedMsg, uint8_t * static uint8_t pack_nr_rx_data_indication_body(nfapi_nr_rx_data_pdu_t *value, uint8_t **ppWritePackedMsg, uint8_t *end) { - AssertFatal(value->pdu_length <= 0xFFFF,"RX_DATA.indication PDU_Length should be within 16 bit, according to SCF222.10.02"); - if(!(push32(value->handle, ppWritePackedMsg, end) && - push16(value->rnti, ppWritePackedMsg, end) && - push8(value->harq_id, ppWritePackedMsg, end) && - push16(value->pdu_length, ppWritePackedMsg, end) && - push8(value->ul_cqi, ppWritePackedMsg, end) && - push16(value->timing_advance, ppWritePackedMsg, end) && - push16(value->rssi, ppWritePackedMsg, end) - )) + if (!(push32(value->handle, ppWritePackedMsg, end) + && push16(value->rnti, ppWritePackedMsg, end) + && push8(value->harq_id, ppWritePackedMsg, end) + && push32(value->pdu_length, ppWritePackedMsg, end) + && push8(value->ul_cqi, ppWritePackedMsg, end) + && push16(value->timing_advance, ppWritePackedMsg, end) + && push16(value->rssi, ppWritePackedMsg, end))) return 0; - if(pusharray8(value->pdu, value->pdu_length, value->pdu_length, ppWritePackedMsg, end) == 0) + if (pusharray8(value->pdu, value->pdu_length, value->pdu_length, ppWritePackedMsg, end) == 0) return 0; return 1; @@ -6066,19 +6064,18 @@ static uint8_t unpack_nr_rx_data_indication_body(nfapi_nr_rx_data_pdu_t *value, uint8_t *end, nfapi_p7_codec_config_t *config) { -if (!(pull32(ppReadPackedMsg, &value->handle, end) && pull16(ppReadPackedMsg, &value->rnti, end) - && pull8(ppReadPackedMsg, &value->harq_id, end) && pull16(ppReadPackedMsg, (uint16_t *)&value->pdu_length, end) - && pull8(ppReadPackedMsg, &value->ul_cqi, end) && pull16(ppReadPackedMsg, &value->timing_advance, end) - && pull16(ppReadPackedMsg, &value->rssi, end))) - return 0; + if (!(pull32(ppReadPackedMsg, &value->handle, end) && pull16(ppReadPackedMsg, &value->rnti, end) + && pull8(ppReadPackedMsg, &value->harq_id, end) && pull32(ppReadPackedMsg, &value->pdu_length, end) + && pull8(ppReadPackedMsg, &value->ul_cqi, end) && pull16(ppReadPackedMsg, &value->timing_advance, end) + && pull16(ppReadPackedMsg, &value->rssi, end))) + return 0; -uint32_t length = value->pdu_length; -value->pdu = nfapi_p7_allocate(sizeof(*value->pdu) * length, config); -if (pullarray8(ppReadPackedMsg, value->pdu, length, length, end) == 0) { - NFAPI_TRACE(NFAPI_TRACE_ERROR, "%s pullarray8 failure\n", __FUNCTION__); - return 0; -} -return 1; + value->pdu = nfapi_p7_allocate(sizeof(*value->pdu) * value->pdu_length, config); + if (pullarray8(ppReadPackedMsg, value->pdu, value->pdu_length, value->pdu_length, end) == 0) { + NFAPI_TRACE(NFAPI_TRACE_ERROR, "%s pullarray8 failure\n", __FUNCTION__); + return 0; + } + return 1; } static uint8_t unpack_nr_rx_data_indication(uint8_t **ppReadPackedMsg, uint8_t *end, nfapi_nr_rx_data_indication_t *msg, nfapi_p7_codec_config_t* config) diff --git a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c index 89c684ee41..a7425067f8 100644 --- a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c +++ b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c @@ -644,7 +644,7 @@ static void _nr_rx_sdu(const module_id_t gnb_mod_idP, const sub_frame_t slotP, const rnti_t rntiP, uint8_t *sduP, - const uint16_t sdu_lenP, + const uint32_t sdu_lenP, const uint16_t timing_advance, const uint8_t ul_cqi, const uint16_t rssi) @@ -939,7 +939,7 @@ void nr_rx_sdu(const module_id_t gnb_mod_idP, const sub_frame_t slotP, const rnti_t rntiP, uint8_t *sduP, - const uint16_t sdu_lenP, + const uint32_t sdu_lenP, const uint16_t timing_advance, const uint8_t ul_cqi, const uint16_t rssi) diff --git a/openair2/LAYER2/NR_MAC_gNB/mac_proto.h b/openair2/LAYER2/NR_MAC_gNB/mac_proto.h index a23e588a49..fe2fec8ab1 100644 --- a/openair2/LAYER2/NR_MAC_gNB/mac_proto.h +++ b/openair2/LAYER2/NR_MAC_gNB/mac_proto.h @@ -343,7 +343,7 @@ void nr_rx_sdu(const module_id_t gnb_mod_idP, const sub_frame_t subframeP, const rnti_t rntiP, uint8_t * sduP, - const uint16_t sdu_lenP, + const uint32_t sdu_lenP, const uint16_t timing_advance, const uint8_t ul_cqi, const uint16_t rssi); From be9b976023f0b0f54a89c21d0985943f2f3076a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Soares=20Silva?= Date: Thu, 11 Jul 2024 14:36:04 +0100 Subject: [PATCH 2/4] Update references of sdu_lenP to uint32_t in _nr_rx_sdu --- openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c index a7425067f8..3e748f4217 100644 --- a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c +++ b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c @@ -725,10 +725,10 @@ static void _nr_rx_sdu(const module_id_t gnb_mod_idP, #if defined(ENABLE_MAC_PAYLOAD_DEBUG) LOG_I(NR_MAC, "Printing received UL MAC payload at gNB side: %d \n"); - for (int i = 0; i < sdu_lenP ; i++) { - //harq_process_ul_ue->a[i] = (unsigned char) rand(); - //printf("a[%d]=0x%02x\n",i,harq_process_ul_ue->a[i]); - printf("%02x ",(unsigned char)sduP[i]); + for (uint32_t i = 0; i < sdu_lenP; i++) { + // harq_process_ul_ue->a[i] = (unsigned char) rand(); + // printf("a[%d]=0x%02x\n",i,harq_process_ul_ue->a[i]); + printf("%02x ", (unsigned char)sduP[i]); } printf("\n"); @@ -770,8 +770,8 @@ static void _nr_rx_sdu(const module_id_t gnb_mod_idP, } else if (sduP) { bool no_sig = true; - for (int k = 0; k < sdu_lenP; k++) { - if(sduP[k] != 0) { + for (uint32_t k = 0; k < sdu_lenP; k++) { + if (sduP[k] != 0) { no_sig = false; break; } @@ -856,7 +856,7 @@ static void _nr_rx_sdu(const module_id_t gnb_mod_idP, } else { LOG_A(NR_MAC, "[RAPROC] RA-Msg3 received (sdu_lenP %d)\n", sdu_lenP); LOG_D(NR_MAC, "[RAPROC] Received Msg3:\n"); - for (int k = 0; k < sdu_lenP; k++) { + for (uint32_t k = 0; k < sdu_lenP; k++) { LOG_D(NR_MAC, "(%i): 0x%x\n", k, sduP[k]); } From 22ba6b751d5c42a7c1713d3fe94c6123f81c3dc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Soares=20Silva?= Date: Thu, 11 Jul 2024 15:00:04 +0100 Subject: [PATCH 3/4] Update sdu_lenP to uint32_t in lcid_crnti_lookahead and nr_process_mac_pdu --- openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c index 3e748f4217..f726ef136b 100644 --- a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c +++ b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c @@ -40,7 +40,7 @@ //#define SRS_IND_DEBUG -static rnti_t lcid_crnti_lookahead(uint8_t *pdu, int pdu_len) +static rnti_t lcid_crnti_lookahead(uint8_t *pdu, uint32_t pdu_len) { uint16_t mac_len = 0; uint16_t mac_subheader_len = 0; @@ -200,7 +200,7 @@ static int nr_process_mac_pdu(instance_t module_idP, frame_t frameP, sub_frame_t slot, uint8_t *pduP, - int pdu_len, + uint32_t pdu_len, const int8_t harq_pid) { From f58765d0237c3200c8ee1d6b55bea566a80f5810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Soares=20Silva?= Date: Thu, 11 Jul 2024 16:44:20 +0100 Subject: [PATCH 4/4] Update sdu_lenP to uint32_t in get_mac_len Add check in lcid_crnti_lookahead to ensure subtraction to pdu_len doesn't cause an underflow --- openair2/LAYER2/NR_MAC_COMMON/nr_mac.h | 13 +++++++------ openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c | 8 +++++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/openair2/LAYER2/NR_MAC_COMMON/nr_mac.h b/openair2/LAYER2/NR_MAC_COMMON/nr_mac.h index 6973f06103..fbd36461e1 100644 --- a/openair2/LAYER2/NR_MAC_COMMON/nr_mac.h +++ b/openair2/LAYER2/NR_MAC_COMMON/nr_mac.h @@ -107,12 +107,13 @@ typedef struct { uint8_t R: 2; // octet 1 [7:6] } __attribute__ ((__packed__)) NR_MAC_SUBHEADER_FIXED; -static inline int get_mac_len(uint8_t* pdu, int pdu_len, uint16_t *mac_ce_len, uint16_t *mac_subheader_len) { - if ( pdu_len < (int)sizeof(NR_MAC_SUBHEADER_SHORT)) +static inline int get_mac_len(uint8_t *pdu, uint32_t pdu_len, uint16_t *mac_ce_len, uint16_t *mac_subheader_len) +{ + if (pdu_len < sizeof(NR_MAC_SUBHEADER_SHORT)) return false; - NR_MAC_SUBHEADER_SHORT *s = (NR_MAC_SUBHEADER_SHORT*) pdu; - NR_MAC_SUBHEADER_LONG *l = (NR_MAC_SUBHEADER_LONG*) pdu; - if (s->F && pdu_len < (int)sizeof(NR_MAC_SUBHEADER_LONG)) + NR_MAC_SUBHEADER_SHORT *s = (NR_MAC_SUBHEADER_SHORT *)pdu; + NR_MAC_SUBHEADER_LONG *l = (NR_MAC_SUBHEADER_LONG *)pdu; + if (s->F && pdu_len < sizeof(NR_MAC_SUBHEADER_LONG)) return false; if (s->F) { *mac_subheader_len = sizeof(*l); @@ -123,7 +124,7 @@ static inline int get_mac_len(uint8_t* pdu, int pdu_len, uint16_t *mac_ce_len, u } return true; } - + // BSR MAC CEs // TS 38.321 ch. 6.1.3.1 // Short BSR for a specific logical channel group ID diff --git a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c index f726ef136b..1048be2528 100644 --- a/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c +++ b/openair2/LAYER2/NR_MAC_gNB/gNB_scheduler_ulsch.c @@ -55,7 +55,13 @@ static rnti_t lcid_crnti_lookahead(uint8_t *pdu, uint32_t pdu_len) break; } pdu += mac_len + mac_subheader_len; - pdu_len -= mac_len + mac_subheader_len; + // if pdu_len can have the value subtracted without underflow, we can subtract + if (pdu_len >= mac_len + mac_subheader_len) { + pdu_len -= mac_len + mac_subheader_len; + } else { + // if not, set to 0 to prevent underflow + pdu_len = 0; + } } return 0; }