From ccfb9314bc0f248204963d10e60005fed5e46e46 Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Sun, 11 Feb 2018 10:59:56 +0100 Subject: [PATCH 1/6] fix various coverity bugs --- lib/src/common/logger_file.cc | 2 +- lib/src/phy/utils/ringbuffer.c | 2 ++ lib/src/upper/rlc_am.cc | 2 +- lib/test/upper/rlc_am_stress_test.cc | 1 + srsue/hdr/upper/rrc.h | 12 +++++++++--- srsue/src/upper/rrc.cc | 8 ++++++++ 6 files changed, 22 insertions(+), 5 deletions(-) diff --git a/lib/src/common/logger_file.cc b/lib/src/common/logger_file.cc index f7532fdd1..25155da08 100644 --- a/lib/src/common/logger_file.cc +++ b/lib/src/common/logger_file.cc @@ -54,7 +54,7 @@ void logger_file::init(std::string file, int max_length_) { pthread_mutex_init(&mutex, NULL); pthread_cond_init(¬_empty, NULL); pthread_cond_init(¬_full, NULL); - max_length = max_length_*1024; + max_length = (int64_t)max_length_*1024; name_idx = 0; filename = file; logfile = fopen(filename.c_str(), "w"); diff --git a/lib/src/phy/utils/ringbuffer.c b/lib/src/phy/utils/ringbuffer.c index d5f99fd87..8f0c3edf2 100644 --- a/lib/src/phy/utils/ringbuffer.c +++ b/lib/src/phy/utils/ringbuffer.c @@ -54,6 +54,7 @@ int srslte_ringbuffer_write(srslte_ringbuffer_t *q, void *p, int nof_bytes) int w_bytes = nof_bytes; pthread_mutex_lock(&q->mutex); if (!q->active) { + pthread_mutex_unlock(&q->mutex); return 0; } if (q->count + w_bytes > q->capacity) { @@ -85,6 +86,7 @@ int srslte_ringbuffer_read(srslte_ringbuffer_t *q, void *p, int nof_bytes) pthread_cond_wait(&q->cvar, &q->mutex); } if (!q->active) { + pthread_mutex_unlock(&q->mutex); return 0; } if (nof_bytes + q->rpm > q->capacity) { diff --git a/lib/src/upper/rlc_am.cc b/lib/src/upper/rlc_am.cc index 2c8b1fbfc..e4a6242fa 100644 --- a/lib/src/upper/rlc_am.cc +++ b/lib/src/upper/rlc_am.cc @@ -1085,11 +1085,11 @@ void rlc_am::handle_control_pdu(uint8_t *payload, uint32_t nof_bytes) it = tx_window.find(i); if (it != tx_window.end()) { if(update_vt_a) { - tx_window.erase(it); if(it->second.buf) { pool->deallocate(it->second.buf); it->second.buf = 0; } + tx_window.erase(it); vt_a = (vt_a + 1)%MOD; vt_ms = (vt_ms + 1)%MOD; } diff --git a/lib/test/upper/rlc_am_stress_test.cc b/lib/test/upper/rlc_am_stress_test.cc index d6fb0882d..548feb818 100644 --- a/lib/test/upper/rlc_am_stress_test.cc +++ b/lib/test/upper/rlc_am_stress_test.cc @@ -47,6 +47,7 @@ public: rlc2 = rlc2_; fail_rate = fail_rate_; run_enable = true; + running = false; } void stop() diff --git a/srsue/hdr/upper/rrc.h b/srsue/hdr/upper/rrc.h index 27764bd02..4ac2dfa2f 100644 --- a/srsue/hdr/upper/rrc.h +++ b/srsue/hdr/upper/rrc.h @@ -76,17 +76,22 @@ class cell_t return false; } cell_t() { - this->has_valid_sib1 = false; - this->has_valid_sib2 = false; - this->has_valid_sib3 = false; + srslte_cell_t tmp = {}; + cell_t(tmp, 0, 0); } cell_t(srslte_cell_t phy_cell, uint32_t earfcn, float rsrp) { this->has_valid_sib1 = false; this->has_valid_sib2 = false; this->has_valid_sib3 = false; + this->has_valid_sib13 = false; this->phy_cell = phy_cell; this->rsrp = rsrp; this->earfcn = earfcn; + in_sync = false; + bzero(&sib1, sizeof(sib1)); + bzero(&sib2, sizeof(sib2)); + bzero(&sib3, sizeof(sib3)); + bzero(&sib13, sizeof(sib13)); } uint32_t earfcn; @@ -114,6 +119,7 @@ class rrc { public: rrc(); + ~rrc(); void init(phy_interface_rrc *phy_, mac_interface_rrc *mac_, diff --git a/srsue/src/upper/rrc.cc b/srsue/src/upper/rrc.cc index 24ccbb4c0..ad5cf29b2 100644 --- a/srsue/src/upper/rrc.cc +++ b/srsue/src/upper/rrc.cc @@ -49,12 +49,20 @@ rrc::rrc() :state(RRC_STATE_IDLE) ,drb_up(false) ,sysinfo_index(0) + ,serving_cell(NULL) { n310_cnt = 0; n311_cnt = 0; serving_cell = new cell_t(); } +rrc::~rrc() +{ + if (serving_cell) { + delete(serving_cell); + } +} + static void liblte_rrc_handler(void *ctx, char *str) { rrc *r = (rrc *) ctx; r->liblte_rrc_log(str); From ad703d5758137842933e855104bada4fbc8f680d Mon Sep 17 00:00:00 2001 From: Ismael Gomez Date: Mon, 12 Feb 2018 09:57:37 +0100 Subject: [PATCH 2/6] Add missing include --- srsepc/src/mme/s1ap_nas_transport.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/srsepc/src/mme/s1ap_nas_transport.cc b/srsepc/src/mme/s1ap_nas_transport.cc index 083201ff6..d4f0702de 100644 --- a/srsepc/src/mme/s1ap_nas_transport.cc +++ b/srsepc/src/mme/s1ap_nas_transport.cc @@ -24,6 +24,7 @@ * */ +#include #include "mme/s1ap.h" #include "mme/s1ap_nas_transport.h" #include "srslte/common/security.h" From 237770fcc29cc9b9455ebbc497167c68dc179812 Mon Sep 17 00:00:00 2001 From: Paul Sutton Date: Mon, 12 Feb 2018 12:24:04 +0000 Subject: [PATCH 3/6] Update coverity badge --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1567c7c71..fa4355408 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ srsLTE ======== -[![Coverity Scan Build Status](https://scan.coverity.com/projects/10045/badge.svg)](https://scan.coverity.com/projects/10045) +[![Coverity Scan Build Status](https://scan.coverity.com/projects/4721/badge.svg)](https://scan.coverity.com/projects/4721) srsLTE is a free and open-source LTE software suite developed by SRS (www.softwareradiosystems.com). From 59e425608fea666c3d35e0b4c7aeafa2848e6506 Mon Sep 17 00:00:00 2001 From: Paul Sutton Date: Mon, 12 Feb 2018 12:44:55 +0000 Subject: [PATCH 4/6] Better fix for buffer size issue --- lib/src/upper/rlc_am.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/src/upper/rlc_am.cc b/lib/src/upper/rlc_am.cc index e4a6242fa..26f42af84 100644 --- a/lib/src/upper/rlc_am.cc +++ b/lib/src/upper/rlc_am.cc @@ -587,7 +587,8 @@ int rlc_am::build_segment(uint8_t *payload, uint32_t nof_bytes, rlc_amd_retx_t r rrc->get_rb_name(lcid).c_str(), nof_bytes, head_len); return 0; } - pdu_space = nof_bytes-head_len-2; + + pdu_space = nof_bytes-head_len; if(pdu_space < (retx.so_end-retx.so_start)) retx.so_end = retx.so_start+pdu_space; @@ -603,10 +604,13 @@ int rlc_am::build_segment(uint8_t *payload, uint32_t nof_bytes, rlc_amd_retx_t r if(lower >= retx.so_end) break; + if(pdu_space <= 2) + break; + upper += old_header.li[i]; head_len = rlc_am_packed_length(&new_header); - pdu_space = nof_bytes-head_len-2; + pdu_space = nof_bytes-head_len; if(pdu_space < (retx.so_end-retx.so_start)) retx.so_end = retx.so_start+pdu_space; From 4cf79c1eadd0ffd24d4cb03d4d8106737971c245 Mon Sep 17 00:00:00 2001 From: Paul Sutton Date: Mon, 12 Feb 2018 13:09:31 +0000 Subject: [PATCH 5/6] Initial fix for segment handling in RLC AM --- lib/src/upper/rlc_am.cc | 19 +++++++++++++------ lib/test/upper/rlc_am_stress_test.cc | 6 +++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/src/upper/rlc_am.cc b/lib/src/upper/rlc_am.cc index 26f42af84..72fbbde17 100644 --- a/lib/src/upper/rlc_am.cc +++ b/lib/src/upper/rlc_am.cc @@ -1221,15 +1221,22 @@ void rlc_am::print_rx_segments() bool rlc_am::add_segment_and_check(rlc_amd_rx_pdu_segments_t *pdu, rlc_amd_rx_pdu_t *segment) { - // Ordered insert - std::list::iterator tmpit; - std::list::iterator it = pdu->segments.begin(); - while(it != pdu->segments.end() && it->header.so < segment->header.so) - it++; - pdu->segments.insert(it, *segment); + // Check segment offset + uint32_t n = 0; + if(!pdu->segments.empty()) { + rlc_amd_rx_pdu_t &back = pdu->segments.back(); + n = back.header.so + back.buf->N_bytes; + } + if(segment->header.so != n) { + pool->deallocate(segment->buf); + return false; + } else { + pdu->segments.push_back(*segment); + } // Check for complete uint32_t so = 0; + std::list::iterator it, tmpit; for(it = pdu->segments.begin(); it != pdu->segments.end(); it++) { if(so != it->header.so) return false; diff --git a/lib/test/upper/rlc_am_stress_test.cc b/lib/test/upper/rlc_am_stress_test.cc index 548feb818..cfed3d6e7 100644 --- a/lib/test/upper/rlc_am_stress_test.cc +++ b/lib/test/upper/rlc_am_stress_test.cc @@ -82,7 +82,7 @@ private: if(((float)rand()/RAND_MAX > fail_rate) && read>0) { rlc2->write_pdu(1, pdu->msg, opp_size); } - usleep(1000); + usleep(100); } running = false; } @@ -187,7 +187,7 @@ private: pdu->N_bytes = 1500; pdu->msg[0] = sn++; rlc->write_sdu(1, pdu); - usleep(1000); + usleep(100); } running = false; } @@ -207,7 +207,7 @@ void stress_test() log1.set_hex_limit(-1); log2.set_hex_limit(-1); - float fail_rate = 0.1; + float fail_rate = 0.01; rlc rlc1; rlc rlc2; From e18cb5ba5ba3d3561d5648f17cc2cd0f5663fcaf Mon Sep 17 00:00:00 2001 From: Paul Sutton Date: Mon, 12 Feb 2018 13:42:59 +0000 Subject: [PATCH 6/6] Fixing RLC AM tests, adding extra check for segment handling --- lib/src/upper/rlc_am.cc | 11 +++++++++++ lib/test/upper/rlc_am_stress_test.cc | 12 +++++++++--- lib/test/upper/rlc_am_test.cc | 28 ++++++++++++++-------------- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/lib/src/upper/rlc_am.cc b/lib/src/upper/rlc_am.cc index 72fbbde17..59ce25576 100644 --- a/lib/src/upper/rlc_am.cc +++ b/lib/src/upper/rlc_am.cc @@ -1221,6 +1221,17 @@ void rlc_am::print_rx_segments() bool rlc_am::add_segment_and_check(rlc_amd_rx_pdu_segments_t *pdu, rlc_amd_rx_pdu_t *segment) { + // Check for first segment + if(0 == segment->header.so) { + std::list::iterator it; + for(it = pdu->segments.begin(); it != pdu->segments.end(); it++) { + pool->deallocate(it->buf); + } + pdu->segments.clear(); + pdu->segments.push_back(*segment); + return false; + } + // Check segment offset uint32_t n = 0; if(!pdu->segments.empty()) { diff --git a/lib/test/upper/rlc_am_stress_test.cc b/lib/test/upper/rlc_am_stress_test.cc index cfed3d6e7..dc1540c96 100644 --- a/lib/test/upper/rlc_am_stress_test.cc +++ b/lib/test/upper/rlc_am_stress_test.cc @@ -139,10 +139,12 @@ class rlc_am_tester ,public thread { public: - rlc_am_tester(rlc_interface_pdcp *rlc_){ + rlc_am_tester(rlc_interface_pdcp *rlc_, std::string name_=""){ rlc = rlc_; run_enable = true; running = false; + rx_pdus = 0; + name = name_; } void stop() @@ -164,6 +166,7 @@ public: { assert(lcid == 1); byte_buffer_pool::get_instance()->deallocate(sdu); + std::cout << "rlc_am_tester " << name << " received " << rx_pdus++ << " PDUs" << std::endl; } void write_pdu_bcch_bch(byte_buffer_t *sdu) {} void write_pdu_bcch_dlsch(byte_buffer_t *sdu) {} @@ -194,6 +197,9 @@ private: bool run_enable; bool running; + long rx_pdus; + + std::string name; rlc_interface_pdcp *rlc; }; @@ -212,8 +218,8 @@ void stress_test() rlc rlc1; rlc rlc2; - rlc_am_tester tester1(&rlc1); - rlc_am_tester tester2(&rlc2); + rlc_am_tester tester1(&rlc1, "tester1"); + rlc_am_tester tester2(&rlc2, "tester2"); mac_dummy mac(&rlc1, &rlc2, fail_rate); ue_interface ue; diff --git a/lib/test/upper/rlc_am_test.cc b/lib/test/upper/rlc_am_test.cc index fbd013e3b..0b8c63668 100644 --- a/lib/test/upper/rlc_am_test.cc +++ b/lib/test/upper/rlc_am_test.cc @@ -482,17 +482,17 @@ void resegment_test_1() // Read the retx PDU from RLC1 and force resegmentation byte_buffer_t retx1; - len = rlc1.read_pdu(retx1.msg, 11); // 4 byte header + 5 data + len = rlc1.read_pdu(retx1.msg, 9); // 4 byte header + 5 data retx1.N_bytes = len; // Write the retx PDU to RLC2 rlc2.write_pdu(retx1.msg, retx1.N_bytes); - assert(9 == rlc1.get_buffer_state()); // 4 byte header + 5 data + assert(9 == rlc1.get_buffer_state()); // Read the remaining segment byte_buffer_t retx2; - len = rlc1.read_pdu(retx2.msg, 11); // 4 byte header + 5 data + len = rlc1.read_pdu(retx2.msg, 9); // 4 byte header + 5 data retx2.N_bytes = len; // Write the retx PDU to RLC2 @@ -591,16 +591,16 @@ void resegment_test_2() // Read the retx PDU from RLC1 and force resegmentation byte_buffer_t retx1; - retx1.N_bytes = rlc1.read_pdu(retx1.msg, 18); // 6 byte header + 10 data + retx1.N_bytes = rlc1.read_pdu(retx1.msg, 16); // 6 byte header + 10 data // Write the retx PDU to RLC2 rlc2.write_pdu(retx1.msg, retx1.N_bytes); - assert(16 == rlc1.get_buffer_state()); // 6 byte header + 10 data + assert(16 == rlc1.get_buffer_state()); // Read the remaining segment byte_buffer_t retx2; - retx2.N_bytes = rlc1.read_pdu(retx2.msg, 18); // 6 byte header + 10 data + retx2.N_bytes = rlc1.read_pdu(retx2.msg, 16); // 6 byte header + 10 data // Write the retx PDU to RLC2 rlc2.write_pdu(retx2.msg, retx2.N_bytes); @@ -696,14 +696,14 @@ void resegment_test_3() // Read the retx PDU from RLC1 and force resegmentation byte_buffer_t retx1; - retx1.N_bytes = rlc1.read_pdu(retx1.msg, 16); // 4 byte header + 10 data + retx1.N_bytes = rlc1.read_pdu(retx1.msg, 14); // 4 byte header + 10 data // Write the retx PDU to RLC2 rlc2.write_pdu(retx1.msg, retx1.N_bytes); // Read the remaining segment byte_buffer_t retx2; - retx2.N_bytes = rlc1.read_pdu(retx2.msg, 16); // 4 byte header + 10 data + retx2.N_bytes = rlc1.read_pdu(retx2.msg, 14); // 4 byte header + 10 data // Write the retx PDU to RLC2 rlc2.write_pdu(retx2.msg, retx2.N_bytes); @@ -799,14 +799,14 @@ void resegment_test_4() // Read the retx PDU from RLC1 and force resegmentation byte_buffer_t retx1; - retx1.N_bytes = rlc1.read_pdu(retx1.msg, 23); // 6 byte header + 15 data + retx1.N_bytes = rlc1.read_pdu(retx1.msg, 21); // 6 byte header + 15 data // Write the retx PDU to RLC2 rlc2.write_pdu(retx1.msg, retx1.N_bytes); // Read the remaining segment byte_buffer_t retx2; - retx2.N_bytes = rlc1.read_pdu(retx2.msg, 23); // 6 byte header + 15 data + retx2.N_bytes = rlc1.read_pdu(retx2.msg, 21); // 6 byte header + 15 data // Write the retx PDU to RLC2 rlc2.write_pdu(retx2.msg, retx2.N_bytes); @@ -902,14 +902,14 @@ void resegment_test_5() // Read the retx PDU from RLC1 and force resegmentation byte_buffer_t retx1; - retx1.N_bytes = rlc1.read_pdu(retx1.msg, 29); // 7 byte header + 20 data + retx1.N_bytes = rlc1.read_pdu(retx1.msg, 27); // 7 byte header + 20 data // Write the retx PDU to RLC2 rlc2.write_pdu(retx1.msg, retx1.N_bytes); // Read the remaining segment byte_buffer_t retx2; - retx2.N_bytes = rlc1.read_pdu(retx2.msg, 29); // 7 byte header + 20 data + retx2.N_bytes = rlc1.read_pdu(retx2.msg, 27); // 7 byte header + 20 data // Write the retx PDU to RLC2 rlc2.write_pdu(retx2.msg, retx2.N_bytes); @@ -1023,11 +1023,11 @@ void resegment_test_6() // Write the retx PDU to RLC2 rlc2.write_pdu(retx1.msg, retx1.N_bytes); - assert(157 == rlc1.get_buffer_state()); + assert(155 == rlc1.get_buffer_state()); // Read the remaining segment byte_buffer_t retx2; - len = rlc1.read_pdu(retx2.msg, 159); + len = rlc1.read_pdu(retx2.msg, 157); retx2.N_bytes = len; // Write the retx PDU to RLC2