From b50672b0ef1da4903c3b327f41f6eb935bce6b93 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Wed, 28 Mar 2018 16:41:10 +0200 Subject: [PATCH 1/4] fix non-returning function warning --- srsue/src/upper/nas.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/srsue/src/upper/nas.cc b/srsue/src/upper/nas.cc index 7069f4a23..939db1686 100644 --- a/srsue/src/upper/nas.cc +++ b/srsue/src/upper/nas.cc @@ -376,6 +376,7 @@ bool nas::integrity_check(byte_buffer_t *pdu) return true; } else { nas_log->error("Invalid integrity check PDU size (%d)\n", pdu->N_bytes); + return false; } } From bf25a5a05cfbc56128e2bbd1df00a1c3da18ea52 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Wed, 28 Mar 2018 16:46:40 +0200 Subject: [PATCH 2/4] fix handling of log vector in eNB --- srsenb/hdr/enb.h | 2 +- srsenb/hdr/phy/phy.h | 5 +++-- srsenb/src/enb.cc | 5 ++++- srsenb/src/phy/phy.cc | 10 +++++----- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/srsenb/hdr/enb.h b/srsenb/hdr/enb.h index ff314977a..6d2a78b44 100644 --- a/srsenb/hdr/enb.h +++ b/srsenb/hdr/enb.h @@ -188,7 +188,7 @@ private: srslte::logger *logger; srslte::log_filter rf_log; - std::vector phy_log; + std::vector phy_log; srslte::log_filter mac_log; srslte::log_filter rlc_log; srslte::log_filter pdcp_log; diff --git a/srsenb/hdr/phy/phy.h b/srsenb/hdr/phy/phy.h index a1616c268..99b23de0c 100644 --- a/srsenb/hdr/phy/phy.h +++ b/srsenb/hdr/phy/phy.h @@ -28,6 +28,7 @@ #define ENBPHY_H #include "srslte/common/log.h" +#include "srslte/common/log_filter.h" #include "phy/txrx.h" #include "phy/phch_worker.h" #include "phy/phch_common.h" @@ -54,8 +55,8 @@ class phy : public phy_interface_mac, public: phy(); - bool init(phy_args_t *args, phy_cfg_t *common_cfg, srslte::radio *radio_handler, mac_interface_phy *mac, srslte::log* log_h); - bool init(phy_args_t *args, phy_cfg_t *common_cfg, srslte::radio *radio_handler, mac_interface_phy *mac, std::vector log_vec); + bool init(phy_args_t *args, phy_cfg_t *common_cfg, srslte::radio *radio_handler, mac_interface_phy *mac, srslte::log_filter* log_h); + bool init(phy_args_t *args, phy_cfg_t *common_cfg, srslte::radio *radio_handler, mac_interface_phy *mac, std::vector log_vec); void stop(); /* MAC->PHY interface */ diff --git a/srsenb/src/enb.cc b/srsenb/src/enb.cc index 564775e5e..6e5e06669 100644 --- a/srsenb/src/enb.cc +++ b/srsenb/src/enb.cc @@ -67,6 +67,9 @@ enb::enb() : started(false) { enb::~enb() { + for (uint32_t i = 0; i < phy_log.size(); i++) { + delete (phy_log[i]); + } } bool enb::init(all_args_t *args_) @@ -89,7 +92,7 @@ bool enb::init(all_args_t *args_) char tmp[16]; sprintf(tmp, "PHY%d",i); mylog->init(tmp, logger, true); - phy_log.push_back((void*) mylog); + phy_log.push_back(mylog); } mac_log.init("MAC ", logger, true); rlc_log.init("RLC ", logger); diff --git a/srsenb/src/phy/phy.cc b/srsenb/src/phy/phy.cc index d47998fa9..08b89d77f 100644 --- a/srsenb/src/phy/phy.cc +++ b/srsenb/src/phy/phy.cc @@ -92,12 +92,12 @@ void phy::parse_config(phy_cfg_t* cfg) bool phy::init(phy_args_t *args, phy_cfg_t *cfg, srslte::radio* radio_handler_, - mac_interface_phy *mac, - srslte::log* log_h) + mac_interface_phy *mac, + srslte::log_filter* log_h) { - std::vector log_vec; + std::vector log_vec; for (int i=0;inof_phy_threads;i++) { - log_vec.push_back((void*)log_h); + log_vec.push_back(log_h); } init(args, cfg, radio_handler_, mac, log_vec); return true; @@ -107,7 +107,7 @@ bool phy::init(phy_args_t *args, phy_cfg_t *cfg, srslte::radio* radio_handler_, mac_interface_phy *mac, - std::vector log_vec) + std::vector log_vec) { mlockall(MCL_CURRENT | MCL_FUTURE); From fffda82f1ee3c48175274e2ff67fcecc244ef06c Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Fri, 23 Mar 2018 16:41:37 +0100 Subject: [PATCH 3/4] increase size of bytebuffer to compensate header, add helper to get remaining size --- lib/include/srslte/common/common.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/include/srslte/common/common.h b/lib/include/srslte/common/common.h index 8089634b7..de9f6d5aa 100644 --- a/lib/include/srslte/common/common.h +++ b/lib/include/srslte/common/common.h @@ -147,6 +147,11 @@ public: { return msg-buffer; } + // Returns the remaining space from what is reported to be the length of msg + uint32_t get_tailroom() + { + return (sizeof(buffer) - (msg-buffer) - N_bytes); + } long get_latency_us() { #ifdef ENABLE_TIMESTAMP From 6063888cc5d7435f3f597cf0938154e22c63099a Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Tue, 27 Mar 2018 21:33:26 +0200 Subject: [PATCH 4/4] protect memcpy's in rx sdu reassembly with boundary checks --- lib/src/upper/rlc_am.cc | 75 +++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 26 deletions(-) diff --git a/lib/src/upper/rlc_am.cc b/lib/src/upper/rlc_am.cc index 5c7cb72ea..d100508d1 100644 --- a/lib/src/upper/rlc_am.cc +++ b/lib/src/upper/rlc_am.cc @@ -128,10 +128,10 @@ void rlc_am::reset() reordering_timeout.reset(); if(tx_sdu) { pool->deallocate(tx_sdu); - tx_sdu = NULL; } - if(rx_sdu) - rx_sdu->reset(); + if(rx_sdu) { + pool->deallocate(rx_sdu); + } vt_a = 0; vt_ms = RLC_AM_WINDOW_SIZE; @@ -827,6 +827,12 @@ int rlc_am::build_data_pdu(uint8_t *payload, uint32_t nof_bytes) rrc->get_rb_name(lcid).c_str(), to_move, pdu_space, head_len); } + // Make sure, at least one SDU (segment) has been added until this point + if (pdu->N_bytes == 0) { + log->error("Generated empty RLC PDU.\n"); + return 0; + } + if(tx_sdu) header.fi |= RLC_FI_FIELD_NOT_END_ALIGNED; // Last byte does not correspond to last byte of SDU @@ -848,7 +854,6 @@ int rlc_am::build_data_pdu(uint8_t *payload, uint32_t nof_bytes) // Set SN header.sn = vt_s; vt_s = (vt_s + 1)%MOD; - log->info("%s PDU scheduled for tx. SN: %d (%d B)\n", rrc->get_rb_name(lcid).c_str(), header.sn, pdu->N_bytes); // Place PDU in tx_window, write header and TX tx_window[header.sn].buf = pdu; @@ -859,6 +864,7 @@ int rlc_am::build_data_pdu(uint8_t *payload, uint32_t nof_bytes) uint8_t *ptr = payload; rlc_am_write_data_pdu_header(&header, &ptr); memcpy(ptr, pdu->msg, pdu->N_bytes); + log->info_hex(payload, pdu->N_bytes, "%s PDU scheduled for tx. SN: %d (%d B)\n", rrc->get_rb_name(lcid).c_str(), header.sn, pdu->N_bytes); debug_state(); return (ptr-payload) + pdu->N_bytes; @@ -868,8 +874,8 @@ void rlc_am::handle_data_pdu(uint8_t *payload, uint32_t nof_bytes, rlc_amd_pdu_h { std::map::iterator it; - log->info_hex(payload, nof_bytes, "%s Rx data PDU SN: %d", - rrc->get_rb_name(lcid).c_str(), header.sn); + log->info_hex(payload, nof_bytes, "%s Rx data PDU SN: %d (%d B), %s", + rrc->get_rb_name(lcid).c_str(), header.sn, nof_bytes, rlc_fi_field_text[header.fi]); if(!inside_rx_window(header.sn)) { if(header.p) { @@ -1157,38 +1163,55 @@ void rlc_am::reassemble_rx_sdus() #endif } } + // Iterate through rx_window, assembling and delivering SDUs while(rx_window.end() != rx_window.find(vr_r)) { // Handle any SDU segments for(uint32_t i=0; imsg[rx_sdu->N_bytes], rx_window[vr_r].buf->msg, len); - rx_sdu->N_bytes += len; - rx_window[vr_r].buf->msg += len; - rx_window[vr_r].buf->N_bytes -= len; - log->info_hex(rx_sdu->msg, rx_sdu->N_bytes, "%s Rx SDU", rrc->get_rb_name(lcid).c_str()); - rx_sdu->set_timestamp(); - pdcp->write_pdu(lcid, rx_sdu); - rx_sdu = pool_allocate; - if (!rx_sdu) { + uint32_t len = rx_window[vr_r].header.li[i]; + if (rx_sdu->get_tailroom() >= len) { + memcpy(&rx_sdu->msg[rx_sdu->N_bytes], rx_window[vr_r].buf->msg, len); + rx_sdu->N_bytes += len; + rx_window[vr_r].buf->msg += len; + rx_window[vr_r].buf->N_bytes -= len; + log->info_hex(rx_sdu->msg, rx_sdu->N_bytes, "%s Rx SDU (%d B)", rrc->get_rb_name(lcid).c_str(), rx_sdu->N_bytes); + rx_sdu->set_timestamp(); + pdcp->write_pdu(lcid, rx_sdu); + + rx_sdu = pool_allocate; + if (!rx_sdu) { #ifdef RLC_AM_BUFFER_DEBUG - log->console("Fatal Error: Could not allocate PDU in reassemble_rx_sdus() (2)\n"); - exit(-1); + log->console("Fatal Error: Could not allocate PDU in reassemble_rx_sdus() (2)\n"); + exit(-1); #else - log->error("Fatal Error: Could not allocate PDU in reassemble_rx_sdus() (2)\n"); - return; + log->error("Fatal Error: Could not allocate PDU in reassemble_rx_sdus() (2)\n"); + return; #endif + } + } else { + log->error("Cannot fit RLC PDU in SDU buffer, dropping both.\n"); + pool->deallocate(rx_sdu); + pool->deallocate(rx_window[vr_r].buf); + rx_window.erase(vr_r); } } // Handle last segment - memcpy(&rx_sdu->msg[rx_sdu->N_bytes], rx_window[vr_r].buf->msg, rx_window[vr_r].buf->N_bytes); - rx_sdu->N_bytes += rx_window[vr_r].buf->N_bytes; - if(rlc_am_end_aligned(rx_window[vr_r].header.fi)) - { - log->info_hex(rx_sdu->msg, rx_sdu->N_bytes, "%s Rx SDU", rrc->get_rb_name(lcid).c_str()); + uint32_t len = rx_window[vr_r].buf->N_bytes; + if (rx_sdu->get_tailroom() >= len) { + memcpy(&rx_sdu->msg[rx_sdu->N_bytes], rx_window[vr_r].buf->msg, len); + rx_sdu->N_bytes += rx_window[vr_r].buf->N_bytes; + } else { + log->error("Cannot fit RLC PDU in SDU buffer, dropping both.\n"); + pool->deallocate(rx_sdu); + pool->deallocate(rx_window[vr_r].buf); + rx_window.erase(vr_r); + } + + if(rlc_am_end_aligned(rx_window[vr_r].header.fi)) { + log->info_hex(rx_sdu->msg, rx_sdu->N_bytes, "%s Rx SDU (%d B)", rrc->get_rb_name(lcid).c_str(), rx_sdu->N_bytes); rx_sdu->set_timestamp(); pdcp->write_pdu(lcid, rx_sdu); rx_sdu = pool_allocate; @@ -1250,7 +1273,7 @@ void rlc_am::print_rx_segments() for(it=rx_segments.begin();it!=rx_segments.end();it++) { std::list::iterator segit; for(segit = it->second.segments.begin(); segit != it->second.segments.end(); segit++) { - ss << " SN:" << segit->header.sn << " SO:" << segit->header.so << " N:" << segit->buf->N_bytes << std::endl; + ss << " SN:" << segit->header.sn << " SO:" << segit->header.so << " N:" << segit->buf->N_bytes << " N_li: " << segit->header.N_li << std::endl; } } log->debug("%s\n", ss.str().c_str());