From c2af3daa9fa21ccc7335fa81729c305acf6e3b39 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Wed, 31 Jan 2018 21:09:12 -0600 Subject: [PATCH] BIT-1854: fix the 'tcp_excessive_data_without_further_acks' option This previously checked against the amount of out-of-sequence data being buffered by the reassembler. It now checks against the total size of all blocks being buffered in the reassembler, which, by nature of still being buffered there, means it's not been acked yet. --- src/Reassem.cc | 33 +++++++++++--------- src/Reassem.h | 14 +++++++-- src/analyzer/protocol/tcp/TCP_Reassembler.cc | 2 +- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/Reassem.cc b/src/Reassem.cc index 14d894be4f..614805ae24 100644 --- a/src/Reassem.cc +++ b/src/Reassem.cc @@ -10,10 +10,11 @@ static const bool DEBUG_reassem = false; -DataBlock::DataBlock(const u_char* data, uint64 size, uint64 arg_seq, - DataBlock* arg_prev, DataBlock* arg_next, - ReassemblerType reassem_type) +DataBlock::DataBlock(DataBlockListInfo* list_info, const u_char* data, + uint64 size, uint64 arg_seq, DataBlock* arg_prev, + DataBlock* arg_next, ReassemblerType reassem_type) { + info = list_info; seq = arg_seq; upper = seq + size; block = new u_char[size]; @@ -31,18 +32,19 @@ DataBlock::DataBlock(const u_char* data, uint64 size, uint64 arg_seq, rtype = reassem_type; Reassembler::sizes[rtype] += pad_size(size) + padded_sizeof(DataBlock); Reassembler::total_size += pad_size(size) + padded_sizeof(DataBlock); + info->totalSize += size; } uint64 Reassembler::total_size = 0; uint64 Reassembler::sizes[REASSEM_NUM]; Reassembler::Reassembler(uint64 init_seq, ReassemblerType reassem_type) + : block_list_info(), + blocks(), last_block(), old_blocks(), last_old_block(), + last_reassem_seq(init_seq), trim_seq(init_seq), + max_old_blocks(0), total_old_blocks(0), + rtype(reassem_type) { - blocks = last_block = 0; - old_blocks = last_old_block = 0; - total_old_blocks = max_old_blocks = 0; - trim_seq = last_reassem_seq = init_seq; - rtype = reassem_type; } Reassembler::~Reassembler() @@ -116,7 +118,7 @@ void Reassembler::NewBlock(double t, uint64 seq, uint64 len, const u_char* data) if ( ! blocks ) blocks = last_block = start_block = - new DataBlock(data, len, seq, 0, 0, rtype); + new DataBlock(&block_list_info, data, len, seq, 0, 0, rtype); else start_block = AddAndCheck(blocks, seq, upper_seq, data); @@ -280,8 +282,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper, // Special check for the common case of appending to the end. if ( last_block && seq == last_block->upper ) { - last_block = new DataBlock(data, upper - seq, seq, - last_block, 0, rtype); + last_block = new DataBlock(&block_list_info, data, upper - seq, + seq, last_block, 0, rtype); return last_block; } @@ -294,7 +296,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper, { // b is the last block, and it comes completely before // the new block. - last_block = new DataBlock(data, upper - seq, seq, b, 0, rtype); + last_block = new DataBlock(&block_list_info, data, upper - seq, + seq, b, 0, rtype); return last_block; } @@ -303,7 +306,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper, if ( upper <= b->seq ) { // The new block comes completely before b. - new_b = new DataBlock(data, upper - seq, seq, b->prev, b, rtype); + new_b = new DataBlock(&block_list_info, data, upper - seq, seq, + b->prev, b, rtype); if ( b == blocks ) blocks = new_b; return new_b; @@ -314,7 +318,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper, { // The new block has a prefix that comes before b. uint64 prefix_len = b->seq - seq; - new_b = new DataBlock(data, prefix_len, seq, b->prev, b, rtype); + new_b = new DataBlock(&block_list_info, data, prefix_len, seq, + b->prev, b, rtype); if ( b == blocks ) blocks = new_b; diff --git a/src/Reassem.h b/src/Reassem.h index 1672a4f9dd..993969c08a 100644 --- a/src/Reassem.h +++ b/src/Reassem.h @@ -18,11 +18,16 @@ enum ReassemblerType { REASSEM_NUM, }; +struct DataBlockListInfo { + uint64 totalSize; +}; + class DataBlock { public: - DataBlock(const u_char* data, uint64 size, uint64 seq, - DataBlock* prev, DataBlock* next, - ReassemblerType reassem_type = REASSEM_UNKNOWN); + DataBlock(DataBlockListInfo* list_info, const u_char* data, + uint64 size, uint64 seq, + DataBlock* prev, DataBlock* next, + ReassemblerType reassem_type = REASSEM_UNKNOWN); ~DataBlock(); @@ -33,6 +38,7 @@ public: uint64 seq, upper; u_char* block; ReassemblerType rtype; + DataBlockListInfo* info; }; class Reassembler : public BroObj { @@ -86,6 +92,7 @@ protected: void CheckOverlap(DataBlock *head, DataBlock *tail, uint64 seq, uint64 len, const u_char* data); + DataBlockListInfo block_list_info; DataBlock* blocks; DataBlock* last_block; @@ -105,6 +112,7 @@ protected: inline DataBlock::~DataBlock() { + info->totalSize -= Size(); Reassembler::total_size -= pad_size(upper - seq) + padded_sizeof(DataBlock); Reassembler::sizes[rtype] -= pad_size(upper - seq) + padded_sizeof(DataBlock); delete [] block; diff --git a/src/analyzer/protocol/tcp/TCP_Reassembler.cc b/src/analyzer/protocol/tcp/TCP_Reassembler.cc index bcbe20d499..f19ace2c39 100644 --- a/src/analyzer/protocol/tcp/TCP_Reassembler.cc +++ b/src/analyzer/protocol/tcp/TCP_Reassembler.cc @@ -501,7 +501,7 @@ int TCP_Reassembler::DataSent(double t, uint64 seq, int len, } if ( tcp_excessive_data_without_further_acks && - NumUndeliveredBytes() > static_cast(tcp_excessive_data_without_further_acks) ) + block_list_info.totalSize > static_cast(tcp_excessive_data_without_further_acks) ) { tcp_analyzer->Weird("excessive_data_without_further_acks"); ClearBlocks();