From 6063888cc5d7435f3f597cf0938154e22c63099a Mon Sep 17 00:00:00 2001 From: Andre Puschmann Date: Tue, 27 Mar 2018 21:33:26 +0200 Subject: [PATCH] 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());