From 2f4767a7d4096f71b182bea65ddca65d7f19a167 Mon Sep 17 00:00:00 2001 From: Michael Cook Date: Mon, 1 Mar 2021 09:10:50 -0800 Subject: [PATCH 1/4] build_oai: Add --sanitize-address option To enable -fsanitize=address option to the compiler which enables run-time checks for some kinds of memory mismanagement. --- cmake_targets/CMakeLists.txt | 5 +++++ cmake_targets/build_oai | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/cmake_targets/CMakeLists.txt b/cmake_targets/CMakeLists.txt index c16170f932..fbc9682d90 100644 --- a/cmake_targets/CMakeLists.txt +++ b/cmake_targets/CMakeLists.txt @@ -333,6 +333,11 @@ else (CUDA_FOUND) ) endif () +if (SANITIZE_ADDRESS) + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address") +endif () + add_definitions("-DASN_DISABLE_OER_SUPPORT") ######################### diff --git a/cmake_targets/build_oai b/cmake_targets/build_oai index 960b241047..dc3faf664e 100755 --- a/cmake_targets/build_oai +++ b/cmake_targets/build_oai @@ -67,6 +67,7 @@ USRP_REC_PLAY="False" BUILD_ECLIPSE=0 NR="False" ITTI_SIM="False" +SANITIZE_ADDRESS="False" OPTIONAL_LIBRARIES="telnetsrv enbscope uescope nrscope msc" trap handle_ctrl_c INT @@ -416,6 +417,9 @@ function main() { CMAKE_CMD="$CMAKE_CMD -GNinja" MAKE_CMD=ninja shift;; + --sanitize-address | -fsanitize=address) + SANITIZE_ADDRESS=True + shift;; --ittiSIM) ittiSIM=1 ITTI_SIM="True" @@ -604,6 +608,7 @@ function main() { echo "set ( USRP_REC_PLAY $USRP_REC_PLAY )" >> $cmake_file echo "set ( SKIP_SHARED_LIB_FLAG $SKIP_SHARED_LIB_FLAG )" >> $cmake_file echo "set ( ITTI_SIM $ITTI_SIM )" >> $cmake_file + echo "set ( SANITIZE_ADDRESS $SANITIZE_ADDRESS )" >> $cmake_file echo 'include(${CMAKE_CURRENT_SOURCE_DIR}/../CMakeLists.txt)' >> $cmake_file cd $DIR/$build_dir/build eval $CMAKE_CMD From 9d1c24d46172357eda675e201b05d9f0f77cb400 Mon Sep 17 00:00:00 2001 From: Cedric Roux Date: Fri, 26 Mar 2021 15:36:43 +0100 Subject: [PATCH 2/4] nr rlc: bugfix: fix NACK with range We do some sanity checks for incoming rlc control packets. One of them is to check that so_start is not > so_end when present. But when 'range' is present and bigger than 1 it means that so_start refers to one PDU and so_end to another one. So we may well have so_start > so_end. This commit fixes that and reorganizes a bit the code to do the check before processing and rejecting the PDU if the values are not correct. (Before this commit we were NACKing the whole PDUs if so_start > so_end.) --- openair2/LAYER2/nr_rlc/nr_rlc_entity_am.c | 45 +++++++++++++-------- openair2/LAYER2/nr_rlc/tests/run_tests.sh | 2 +- openair2/LAYER2/nr_rlc/tests/test15.h | 19 +++++++++ openair2/LAYER2/nr_rlc/tests/test15.txt.gz | Bin 0 -> 554 bytes 4 files changed, 49 insertions(+), 17 deletions(-) create mode 100644 openair2/LAYER2/nr_rlc/tests/test15.h create mode 100644 openair2/LAYER2/nr_rlc/tests/test15.txt.gz diff --git a/openair2/LAYER2/nr_rlc/nr_rlc_entity_am.c b/openair2/LAYER2/nr_rlc/nr_rlc_entity_am.c index 1d4efc1e26..000c764e8f 100644 --- a/openair2/LAYER2/nr_rlc/nr_rlc_entity_am.c +++ b/openair2/LAYER2/nr_rlc/nr_rlc_entity_am.c @@ -650,7 +650,7 @@ control: control_decoder = decoder; control_e1 = e1; while (control_e1) { - nr_rlc_pdu_decoder_get_bits(&control_decoder, entity->sn_field_length); R(control_decoder); /* NACK_SN */ + nack_sn = nr_rlc_pdu_decoder_get_bits(&control_decoder, entity->sn_field_length); R(control_decoder); control_e1 = nr_rlc_pdu_decoder_get_bits(&control_decoder, 1); R(control_decoder); control_e2 = nr_rlc_pdu_decoder_get_bits(&control_decoder, 1); R(control_decoder); control_e3 = nr_rlc_pdu_decoder_get_bits(&control_decoder, 1); R(control_decoder); @@ -660,17 +660,36 @@ control: } else { nr_rlc_pdu_decoder_get_bits(&control_decoder, 1); R(control_decoder); } + /* check range and so_start/so_end consistency */ if (control_e2) { - nr_rlc_pdu_decoder_get_bits(&control_decoder, 16); R(control_decoder); /* SOstart */ - nr_rlc_pdu_decoder_get_bits(&control_decoder, 16); R(control_decoder); /* SOend */ + so_start = nr_rlc_pdu_decoder_get_bits(&control_decoder, 16); R(control_decoder); + so_end = nr_rlc_pdu_decoder_get_bits(&control_decoder, 16); R(control_decoder); + } else { + so_start = 0; + so_end = 0xffff; } if (control_e3) { - nr_rlc_pdu_decoder_get_bits(&control_decoder, 8); R(control_decoder); /* NACK range */ + range = nr_rlc_pdu_decoder_get_bits(&control_decoder, 8); R(control_decoder); + } else { + range = 1; + } + if (range < 1) { + LOG_E(RLC, "%s:%d:%s: error, bad 'range' in RLC NACK (sn %d)\n", + __FILE__, __LINE__, __FUNCTION__, nack_sn); + goto err; + } + /* so_start can be > so_end if more than one range; they don't refer + * to the same PDU then + */ + if (range == 1 && so_end < so_start) { + LOG_E(RLC, "%s:%d:%s: error, bad so start/end (sn %d)\n", + __FILE__, __LINE__, __FUNCTION__, nack_sn); + goto err; } } /* 38.322 5.3.3.3 says to stop t_poll_retransmit if a ACK or NACK is - * received for the SN 'poll_sn' + * received for the SN 'poll_sn' - check ACK case (NACK done below) */ if (sn_compare_tx(entity, entity->poll_sn, ack_sn) < 0) entity->t_poll_retransmit_start = 0; @@ -697,28 +716,22 @@ control: if (e2) { so_start = nr_rlc_pdu_decoder_get_bits(&decoder, 16); R(decoder); so_end = nr_rlc_pdu_decoder_get_bits(&decoder, 16); R(decoder); - if (so_end < so_start) { - LOG_W(RLC, "%s:%d:%s: warning, bad so start/end, NACK the whole PDU (sn %d)\n", - __FILE__, __LINE__, __FUNCTION__, nack_sn); - so_start = 0; - so_end = -1; - } - /* special value 0xffff indicates 'all bytes to the end' */ - if (so_end == 0xffff) - so_end = -1; } else { so_start = 0; - so_end = -1; + so_end = 0xffff; } if (e3) { range = nr_rlc_pdu_decoder_get_bits(&decoder, 8); R(decoder); } else { range = 1; } + /* special value 0xffff indicates 'all bytes to the end' */ + if (so_end == 0xffff) + so_end = -1; process_received_nack(entity, nack_sn, so_start, so_end, range, sn_set); /* 38.322 5.3.3.3 says to stop t_poll_retransmit if a ACK or NACK is - * received for the SN 'poll_sn' + * received for the SN 'poll_sn' - check NACK case (ACK done above) */ if (sn_compare_tx(entity, nack_sn, entity->poll_sn) <= 0 && sn_compare_tx(entity, entity->poll_sn, (nack_sn + range) % entity->sn_modulus) < 0) diff --git a/openair2/LAYER2/nr_rlc/tests/run_tests.sh b/openair2/LAYER2/nr_rlc/tests/run_tests.sh index bf1b343d7e..a98f6b5d0f 100755 --- a/openair2/LAYER2/nr_rlc/tests/run_tests.sh +++ b/openair2/LAYER2/nr_rlc/tests/run_tests.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_count=14 +test_count=15 for i in `seq $test_count` do diff --git a/openair2/LAYER2/nr_rlc/tests/test15.h b/openair2/LAYER2/nr_rlc/tests/test15.h new file mode 100644 index 0000000000..8ce9f97896 --- /dev/null +++ b/openair2/LAYER2/nr_rlc/tests/test15.h @@ -0,0 +1,19 @@ +/* + * am test (SN field size 18): + * test "range" in NACK, generate a case where so_start > so_end + * (so so_start and so_end are not from the same PDU) + */ + +TIME, 1, + GNB_AM, 100000, 100000, 45, 35, 0, -1, -1, 8, 18, + UE_AM, 100000, 100000, 45, 35, 0, -1, -1, 8, 18, + GNB_PDU_SIZE, 40, + UE_PDU_SIZE, 80, + GNB_SDU, 0, 50, + GNB_SDU, 1, 50, + GNB_SDU, 2, 50, +TIME, 2, + UE_RECV_FAILS, 1, +TIME, 4, + UE_RECV_FAILS, 0, +TIME, -1 diff --git a/openair2/LAYER2/nr_rlc/tests/test15.txt.gz b/openair2/LAYER2/nr_rlc/tests/test15.txt.gz new file mode 100644 index 0000000000000000000000000000000000000000..e615a59a5faa88571ac77198d4f53f979163309c GIT binary patch literal 554 zcmV+_0@eK=iwFo!vs_>T19W9`bTKt9ba-?C&6Z1R6hRb5_x_5r)Q!}A)N7VvaOWn; zN+k57S^`02CKJ(L@62RIGMdZ~!6eW$z35w-7qKF}m zL?j^@DM&>+q(=s1L>e-o0TonHLmiE1LNi*>igsv^4(NzBbix8ApAQMhzJMuam}3!3 zSjGxgu@3980UNP~O?bcsSKM&NBcAY#7rf#f-s1y4;tiix>l1~jL?b#e5)&~K3$YSA zVow~1BXI>Ykpf9bO0t6jx-XEN6iJDcNrhBN9jPY`q>RXs#1;W)cB{sm-e;IyX$uG zZCq~YeM7&h48O0Z3(_8+-peOt&DY)GTZixQ`LpBp^290JrZ^fS99zh_%)H}Tikew+0`pU3Cz^V;VD zK9M^{ZTqmAAh!))-&d1J_f748O^XeuJrtRj|Dlb3JOj@)={9@o5AE%X{s-8hA32Z1 sU%gm62EKO-7RzC1i^a6Oy85x(f;PUruRtDKpa% Date: Wed, 31 Mar 2021 16:24:04 +0200 Subject: [PATCH 3/4] fix: do NR only if at least one gNB is connected to the eNB We used to set 'does_nr' of an UE if we detect ENDC supported in the UE capabilities. Then we activate NR measurements if 'does_nr' is true. The problem is that if the eNB is not connected to a gNB but the UE reports some NR measurements (because a gNB is running somewhere near) then the eNB will crash when starting the switch to NR. A (quick) solution is to set 'does_nr' only if there is a gNB connected to the eNB. Maybe not the best solution. To be changed if needed. --- openair2/RRC/LTE/rrc_defs.h | 9 ++++++++- openair2/RRC/LTE/rrc_eNB.c | 31 ++++++++++++++++++------------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/openair2/RRC/LTE/rrc_defs.h b/openair2/RRC/LTE/rrc_defs.h index c19335910b..6615541662 100644 --- a/openair2/RRC/LTE/rrc_defs.h +++ b/openair2/RRC/LTE/rrc_defs.h @@ -67,6 +67,8 @@ #define MAX_NUM_NEIGH_CELLs 6 /* maximum neighbouring cells number */ +#define MAX_NUM_GNB_CELLs 1 /* maximum gNB cells number */ + #define UE_STATE_NOTIFICATION_INTERVAL 50 #define IPV4_ADDR "%u.%u.%u.%u" @@ -804,8 +806,13 @@ typedef struct eNB_RRC_INST_s { int num_neigh_cells_cc[MAX_NUM_CCs]; uint32_t neigh_cells_id[MAX_NUM_NEIGH_CELLs][MAX_NUM_CCs]; + // gNB cells connected to this eNB + int num_gnb_cells; + int num_gnb_cells_cc[MAX_NUM_GNB_CELLs]; + uint32_t gnb_cells_id[MAX_NUM_GNB_CELLs][MAX_NUM_CCs]; + // Nr scc freq band and SSB absolute frequency - uint32_t nr_neigh_freq_band[MAX_NUM_NEIGH_CELLs][MAX_NUM_CCs]; + uint32_t nr_gnb_freq_band[MAX_NUM_GNB_CELLs][MAX_NUM_CCs]; int nr_scg_ssb_freq; // other RAN parameters diff --git a/openair2/RRC/LTE/rrc_eNB.c b/openair2/RRC/LTE/rrc_eNB.c index 526ce129b1..9f0e40806a 100644 --- a/openair2/RRC/LTE/rrc_eNB.c +++ b/openair2/RRC/LTE/rrc_eNB.c @@ -1283,7 +1283,7 @@ rrc_eNB_generate_UECapabilityEnquiry( T(T_ENB_RRC_UE_CAPABILITY_ENQUIRY, T_INT(ctxt_pP->module_id), T_INT(ctxt_pP->frame), T_INT(ctxt_pP->subframe), T_INT(ctxt_pP->rnti)); int16_t eutra_band = RC.rrc[ctxt_pP->module_id]->configuration.eutra_band[0]; - uint32_t nr_band = RC.rrc[ctxt_pP->module_id]->nr_neigh_freq_band[0][0]; + uint32_t nr_band = RC.rrc[ctxt_pP->module_id]->nr_gnb_freq_band[0][0]; size = do_UECapabilityEnquiry( ctxt_pP, buffer, @@ -1333,7 +1333,7 @@ rrc_eNB_generate_NR_UECapabilityEnquiry( T(T_ENB_RRC_UE_CAPABILITY_ENQUIRY, T_INT(ctxt_pP->module_id), T_INT(ctxt_pP->frame), T_INT(ctxt_pP->subframe), T_INT(ctxt_pP->rnti)); int16_t eutra_band = RC.rrc[ctxt_pP->module_id]->configuration.eutra_band[0]; - uint32_t nr_band = RC.rrc[ctxt_pP->module_id]->nr_neigh_freq_band[0][0]; + uint32_t nr_band = RC.rrc[ctxt_pP->module_id]->nr_gnb_freq_band[0][0]; size = do_NR_UECapabilityEnquiry( ctxt_pP, buffer, @@ -7588,8 +7588,8 @@ is_en_dc_supported( ) //----------------------------------------------------------------------------- { - /* to be refined - check that the eNB is connected to a gNB, check that - * the bands supported by the UE include the band of the gNB + /* to be refined - check that the bands supported by the UE include + * the band of the gNB */ #define NCE nonCriticalExtension return c != NULL @@ -8289,8 +8289,13 @@ rrc_eNB_decode_dcch( ue_context_p->ue_context.UE_Capability = 0; } - if (dec_rval.code == RC_OK) - ue_context_p->ue_context.does_nr = is_en_dc_supported(ue_context_p->ue_context.UE_Capability); + if (dec_rval.code == RC_OK) { + /* do NR only if at least one gNB connected */ + if (RC.rrc[ctxt_pP->module_id]->num_gnb_cells != 0) + ue_context_p->ue_context.does_nr = is_en_dc_supported(ue_context_p->ue_context.UE_Capability); + else + ue_context_p->ue_context.does_nr = 0; + } if (EPC_MODE_ENABLED) { rrc_eNB_send_S1AP_UE_CAPABILITIES_IND(ctxt_pP, @@ -9029,22 +9034,22 @@ void rrc_subframe_process(protocol_ctxt_t *const ctxt_pP, const int CC_id) { } void rrc_eNB_process_ENDC_x2_setup_request(int mod_id, x2ap_ENDC_setup_req_t *m) { - if (RC.rrc[mod_id]->num_neigh_cells > MAX_NUM_NEIGH_CELLs) { - LOG_E(RRC, "Error: number of neighbouring cells is exceeded \n"); + if (RC.rrc[mod_id]->num_gnb_cells >= MAX_NUM_GNB_CELLs) { + LOG_E(RRC, "Error: number of gNB cells is exceeded\n"); return; } if (m->num_cc > MAX_NUM_CCs) { - LOG_E(RRC, "Error: number of neighbouring cells carriers is exceeded \n"); + LOG_E(RRC, "Error: number of gNB cells carriers is exceeded \n"); return; } - RC.rrc[mod_id]->num_neigh_cells++; - RC.rrc[mod_id]->num_neigh_cells_cc[RC.rrc[mod_id]->num_neigh_cells-1] = m->num_cc; + RC.rrc[mod_id]->num_gnb_cells++; + RC.rrc[mod_id]->num_gnb_cells_cc[RC.rrc[mod_id]->num_gnb_cells-1] = m->num_cc; for (int i=0; inum_cc; i++) { - RC.rrc[mod_id]->neigh_cells_id[RC.rrc[mod_id]->num_neigh_cells-1][i] = m->Nid_cell[i]; - RC.rrc[mod_id]->nr_neigh_freq_band[RC.rrc[mod_id]->num_neigh_cells-1][i] = m->servedNrCell_band[i]; + RC.rrc[mod_id]->gnb_cells_id[RC.rrc[mod_id]->num_gnb_cells-1][i] = m->Nid_cell[i]; + RC.rrc[mod_id]->nr_gnb_freq_band[RC.rrc[mod_id]->num_gnb_cells-1][i] = m->servedNrCell_band[i]; } } From 6c6b46985ba1e2d5be843b2898349f64165c6c80 Mon Sep 17 00:00:00 2001 From: Cedric Roux Date: Wed, 31 Mar 2021 16:28:47 +0200 Subject: [PATCH 4/4] x2: exit if connection between eNB and gNB breaks Before this commit, when the gNB crashes, the eNB keeps running. Same, if the eNB crashes, the gNB keeps running. In a far past when this happened, the other program (the one not crashing) was forced to exit. Then this behavior was changed, for some reason. But the code was not finished, so now we have a system in an inconsistent state. So either we accept that the connection between eNB and gNB can break, that is one of the programs crashes, and we clean the state of the program that keeps running. But this is a complex work and it will surely not survive very long, because someone will change something in the code later that will break this complex behavior. Or, simpler, we go back to initial behavior, which is: the program that did not crash does actually exit when the other crashes. This commit provides the second solution. It can easily be reverted whenever someone wants to implement the complex solution. --- openair2/X2AP/x2ap_eNB.c | 4 +--- openair2/X2AP/x2ap_eNB_handler.c | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openair2/X2AP/x2ap_eNB.c b/openair2/X2AP/x2ap_eNB.c index 58535646fd..321190015d 100644 --- a/openair2/X2AP/x2ap_eNB.c +++ b/openair2/X2AP/x2ap_eNB.c @@ -116,9 +116,7 @@ void x2ap_eNB_handle_sctp_association_resp(instance_t instance, sctp_new_associa /* some sanity check - to be refined at some point */ if (sctp_new_association_resp->sctp_state != SCTP_STATE_ESTABLISHED) { X2AP_ERROR("x2ap_enb_data_p not NULL and sctp state not SCTP_STATE_ESTABLISHED, what to do?\n"); - // Allow for a gracious exit when we kill first the gNB, then the eNB - //abort(); - return; + exit(1); } x2ap_enb_data_p->in_streams = sctp_new_association_resp->in_streams; diff --git a/openair2/X2AP/x2ap_eNB_handler.c b/openair2/X2AP/x2ap_eNB_handler.c index 1fb28ea05b..d60f7654ad 100644 --- a/openair2/X2AP/x2ap_eNB_handler.c +++ b/openair2/X2AP/x2ap_eNB_handler.c @@ -690,9 +690,11 @@ int x2ap_eNB_handle_x2_setup_failure(instance_t instance, // need a FSM to handle all cases if ((ie->value.choice.Cause.present == X2AP_Cause_PR_misc) && (ie->value.choice.Cause.choice.misc == X2AP_CauseMisc_unspecified)) { - X2AP_WARN("Received X2 setup failure for eNB ... eNB is not ready\n"); + X2AP_ERROR("Received X2 setup failure for eNB ... eNB is not ready\n"); + exit(1); } else { X2AP_ERROR("Received x2 setup failure for eNB... please check your parameters\n"); + exit(1); } x2ap_eNB_data->state = X2AP_ENB_STATE_WAITING;