From: Mike Stepanek (mstepane) <mstepane@cisco.com>
Date: Mon, 14 Dec 2020 21:18:29 +0000 (+0000)
Subject: Merge pull request #2654 in SNORT/snort3 from ~KATHARVE/snort3:http_mem to master
X-Git-Tag: 3.0.3-6~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=43c0fe593dbd457ae6988822c38799f56c787346;p=thirdparty%2Fsnort3.git

Merge pull request #2654 in SNORT/snort3 from ~KATHARVE/snort3:http_mem to master

Squashed commit of the following:

commit 1d1ae0a0c472fd241db960b3463c451271d5bdd5
Author: Katura Harvey <katharve@cisco.com>
Date:   Thu Dec 3 12:34:01 2020 -0500

    http_inspect: explicit memory allocation for transactions and partial inspections
---

diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.cc b/src/service_inspectors/http2_inspect/http2_flow_data.cc
index c766a12b3..e202b2079 100644
--- a/src/service_inspectors/http2_inspect/http2_flow_data.cc
+++ b/src/service_inspectors/http2_inspect/http2_flow_data.cc
@@ -170,14 +170,14 @@ uint32_t Http2FlowData::get_current_stream_id(const HttpCommon::SourceId source_
     return current_stream[source_id];
 }
 
-void Http2FlowData::allocate_hi_memory()
+void Http2FlowData::allocate_hi_memory(HttpFlowData* hi_flow_data)
 {
-    update_allocations(HttpFlowData::get_memory_usage_estimate());
+    update_allocations(hi_flow_data->size_of());
 }
 
-void Http2FlowData::deallocate_hi_memory()
+void Http2FlowData::deallocate_hi_memory(HttpFlowData* hi_flow_data)
 {
-    update_deallocations(HttpFlowData::get_memory_usage_estimate());
+    update_deallocations(hi_flow_data->size_of());
 }
 
 bool Http2FlowData::is_mid_frame() const
diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.h b/src/service_inspectors/http2_inspect/http2_flow_data.h
index 2b2758eae..932dcf2ac 100644
--- a/src/service_inspectors/http2_inspect/http2_flow_data.h
+++ b/src/service_inspectors/http2_inspect/http2_flow_data.h
@@ -199,8 +199,8 @@ private:
 
     // When H2I allocates http_inspect flows, it bypasses the usual FlowData memory allocation
     // bookkeeping. So H2I needs to update memory allocations and deallocations itself.
-    void allocate_hi_memory();
-    void deallocate_hi_memory();
+    void allocate_hi_memory(HttpFlowData* hi_flow_data);
+    void deallocate_hi_memory(HttpFlowData* hi_flow_data);
 };
 
 #endif
diff --git a/src/service_inspectors/http2_inspect/http2_stream.cc b/src/service_inspectors/http2_inspect/http2_stream.cc
index 5aed057ec..26b197a52 100644
--- a/src/service_inspectors/http2_inspect/http2_stream.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream.cc
@@ -46,7 +46,7 @@ Http2Stream::~Http2Stream()
 {
     delete current_frame;
     if (hi_flow_data)
-        session_data->deallocate_hi_memory();
+        session_data->deallocate_hi_memory(hi_flow_data);
     delete hi_flow_data;
 }
 
@@ -80,9 +80,9 @@ void Http2Stream::clear_frame()
     if ((state[SRC_CLIENT] >= STREAM_COMPLETE) && (state[SRC_SERVER] >= STREAM_COMPLETE) &&
         (hi_flow_data != nullptr))
     {
+        session_data->deallocate_hi_memory(hi_flow_data);
         delete hi_flow_data;
         hi_flow_data = nullptr;
-        session_data->deallocate_hi_memory();
     }
 }
 
@@ -99,7 +99,7 @@ void Http2Stream::set_hi_flow_data(HttpFlowData* flow_data)
 {
     assert(hi_flow_data == nullptr);
     hi_flow_data = flow_data;
-    session_data->allocate_hi_memory();
+    session_data->allocate_hi_memory(hi_flow_data);
 }
 
 const Field& Http2Stream::get_buf(unsigned id)
diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h
index e8523bb40..c1202247b 100755
--- a/src/service_inspectors/http_inspect/http_enum.h
+++ b/src/service_inspectors/http_inspect/http_enum.h
@@ -62,7 +62,7 @@ enum PEG_COUNT { PEG_FLOW = 0, PEG_SCAN, PEG_REASSEMBLE, PEG_INSPECT, PEG_REQUES
     PEG_OTHER_METHOD, PEG_REQUEST_BODY, PEG_CHUNKED, PEG_URI_NORM, PEG_URI_PATH, PEG_URI_CODING,
     PEG_CONCURRENT_SESSIONS, PEG_MAX_CONCURRENT_SESSIONS, PEG_DETAINED, PEG_SCRIPT_DETECTION,
     PEG_PARTIAL_INSPECT, PEG_EXCESS_PARAMS, PEG_PARAMS, PEG_CUTOVERS, PEG_SSL_SEARCH_ABND_EARLY,
-    PEG_COUNT_MAX };
+    PEG_PIPELINED_FLOWS, PEG_PIPELINED_REQUESTS, PEG_COUNT_MAX };
 
 // Result of scanning by splitter
 enum ScanResult { SCAN_NOT_FOUND, SCAN_NOT_FOUND_ACCELERATE, SCAN_FOUND, SCAN_FOUND_PIECE,
diff --git a/src/service_inspectors/http_inspect/http_flow_data.cc b/src/service_inspectors/http_inspect/http_flow_data.cc
index f505e02dc..12ea0ff2f 100644
--- a/src/service_inspectors/http_inspect/http_flow_data.cc
+++ b/src/service_inspectors/http_inspect/http_flow_data.cc
@@ -46,10 +46,6 @@ unsigned HttpFlowData::inspector_id = 0;
 uint64_t HttpFlowData::instance_count = 0;
 #endif
 
-const uint16_t HttpFlowData::memory_usage_estimate = sizeof(HttpFlowData) + sizeof(HttpTransaction)
-    + sizeof(HttpMsgRequest) + sizeof(HttpMsgStatus) + (2 * sizeof(HttpMsgHeader)) + sizeof(HttpUri)
-    + header_size_estimate + small_things;
-
 HttpFlowData::HttpFlowData() : FlowData(inspector_id)
 {
 #ifdef REG_TEST
@@ -89,7 +85,9 @@ HttpFlowData::~HttpFlowData()
         delete events[k];
         delete[] section_buffer[k];
         delete[] partial_buffer[k];
+        update_deallocations(partial_buffer_length[k]);
         delete[] partial_detect_buffer[k];
+        update_deallocations(partial_detect_length[k]);
         HttpTransaction::delete_transaction(transaction[k], nullptr);
         delete cutter[k];
         if (compress_stream[k] != nullptr)
@@ -116,6 +114,11 @@ HttpFlowData::~HttpFlowData()
     }
 }
 
+size_t HttpFlowData::size_of()
+{
+    return sizeof(HttpFlowData) + (2 * sizeof(HttpEventGen));
+}
+
 void HttpFlowData::half_reset(SourceId source_id)
 {
     assert((source_id == SRC_CLIENT) || (source_id == SRC_SERVER));
@@ -206,6 +209,7 @@ bool HttpFlowData::add_to_pipeline(HttpTransaction* latest)
     if (pipeline == nullptr)
     {
         pipeline = new HttpTransaction*[MAX_PIPELINE];
+        HttpModule::increment_peg_counts(PEG_PIPELINED_FLOWS);
     }
     assert(!pipeline_overflow && !pipeline_underflow);
     int new_back = (pipeline_back+1) % MAX_PIPELINE;
@@ -216,6 +220,7 @@ bool HttpFlowData::add_to_pipeline(HttpTransaction* latest)
     }
     pipeline[pipeline_back] = latest;
     pipeline_back = new_back;
+    HttpModule::increment_peg_counts(PEG_PIPELINED_REQUESTS);
     return true;
 }
 
@@ -249,11 +254,6 @@ HttpInfractions* HttpFlowData::get_infractions(SourceId source_id)
     return transaction[source_id]->get_infractions(source_id);
 }
 
-uint16_t HttpFlowData::get_memory_usage_estimate()
-{
-    return memory_usage_estimate;
-}
-
 void HttpFlowData::finish_h2_body(HttpCommon::SourceId source_id, HttpEnums::H2BodyState state,
     bool clear_partial_buffer)
 {
@@ -264,14 +264,18 @@ void HttpFlowData::finish_h2_body(HttpCommon::SourceId source_id, HttpEnums::H2B
     {
         // We've already sent all data through detection so no need to reinspect. Just need to
         // prep for trailers
+        update_deallocations(partial_buffer_length[source_id]);
         partial_buffer_length[source_id] = 0;
         delete[] partial_buffer[source_id];
         partial_buffer[source_id] = nullptr;
-        body_octets[source_id] += partial_inspected_octets[source_id];
-        partial_inspected_octets[source_id] = 0;
+
+        update_deallocations(partial_detect_length[source_id]);
         partial_detect_length[source_id] = 0;
         delete[] partial_detect_buffer[source_id];
         partial_detect_buffer[source_id] = nullptr;
+
+        body_octets[source_id] += partial_inspected_octets[source_id];
+        partial_inspected_octets[source_id] = 0;
     }
 }
 
diff --git a/src/service_inspectors/http_inspect/http_flow_data.h b/src/service_inspectors/http_inspect/http_flow_data.h
index a88aa3a91..8f8ef970e 100644
--- a/src/service_inspectors/http_inspect/http_flow_data.h
+++ b/src/service_inspectors/http_inspect/http_flow_data.h
@@ -46,7 +46,7 @@ public:
     ~HttpFlowData() override;
     static unsigned inspector_id;
     static void init() { inspector_id = snort::FlowData::create_flow_data_id(); }
-    size_t size_of() override { return sizeof(*this); }
+    size_t size_of() override;
 
     friend class HttpInspect;
     friend class HttpMsgSection;
@@ -77,8 +77,6 @@ public:
     void reset_partial_flush(HttpCommon::SourceId source_id)
     { partial_flush[source_id] = false; }
 
-    static uint16_t get_memory_usage_estimate();
-
 private:
     // HTTP/2 handling
     bool for_http2 = false;
@@ -192,12 +190,6 @@ private:
     // Transactions with uncleared sections awaiting deletion
     HttpTransaction* discard_list = nullptr;
 
-    // Estimates of how much memory http_inspect uses to process a stream for H2I
-    static const uint16_t header_size_estimate = 3000;  // combined raw size of request and
-                                                        //response message headers
-    static const uint16_t small_things = 400; // minor memory costs not otherwise accounted for
-    static const uint16_t memory_usage_estimate;
-
 #ifdef REG_TEST
     static uint64_t instance_count;
     uint64_t seq_num;
diff --git a/src/service_inspectors/http_inspect/http_msg_body.cc b/src/service_inspectors/http_inspect/http_msg_body.cc
index f19395fe0..253dab51e 100644
--- a/src/service_inspectors/http_inspect/http_msg_body.cc
+++ b/src/service_inspectors/http_inspect/http_msg_body.cc
@@ -92,6 +92,7 @@ void HttpMsgBody::analyze()
         }
 
         delete[] partial_detect_buffer;
+        session_data->update_deallocations(partial_detect_length);
 
         if (!session_data->partial_flush[source_id])
         {
@@ -105,6 +106,7 @@ void HttpMsgBody::analyze()
             memcpy(save_partial, detect_data.start(), detect_data.length());
             partial_detect_buffer = save_partial;
             partial_detect_length = detect_data.length();
+            session_data->update_allocations(partial_detect_length);
         }
 
         set_file_data(const_cast<uint8_t*>(detect_data.start()),
diff --git a/src/service_inspectors/http_inspect/http_msg_head_shared.cc b/src/service_inspectors/http_inspect/http_msg_head_shared.cc
index 002407053..8bf81aefd 100755
--- a/src/service_inspectors/http_inspect/http_msg_head_shared.cc
+++ b/src/service_inspectors/http_inspect/http_msg_head_shared.cc
@@ -34,6 +34,15 @@ using namespace HttpCommon;
 using namespace HttpEnums;
 using namespace snort;
 
+HttpMsgHeadShared::HttpMsgHeadShared(const uint8_t* buffer, const uint16_t buf_size, HttpFlowData* session_data_,
+    HttpCommon::SourceId source_id_, bool buf_owner, snort::Flow* flow_,
+    const HttpParaList* params_): HttpMsgSection(buffer, buf_size, session_data_, source_id_,
+    buf_owner, flow_, params_), own_msg_buffer(buf_owner)
+{
+    if (own_msg_buffer)
+        session_data->update_allocations(msg_text.length());
+}
+
 HttpMsgHeadShared::~HttpMsgHeadShared()
 {
     delete[] header_line;
@@ -47,6 +56,9 @@ HttpMsgHeadShared::~HttpMsgHeadShared()
         list_ptr = list_ptr->next;
         delete temp_ptr;
     }
+
+    if (own_msg_buffer)
+        session_data->update_deallocations(msg_text.length());
 }
 
 // All the header processing that is done for every message (i.e. not just-in-time) is done here.
diff --git a/src/service_inspectors/http_inspect/http_msg_head_shared.h b/src/service_inspectors/http_inspect/http_msg_head_shared.h
index fe06b4d96..8b3ba4ef6 100755
--- a/src/service_inspectors/http_inspect/http_msg_head_shared.h
+++ b/src/service_inspectors/http_inspect/http_msg_head_shared.h
@@ -60,9 +60,7 @@ public:
 protected:
     HttpMsgHeadShared(const uint8_t* buffer, const uint16_t buf_size,
         HttpFlowData* session_data_, HttpCommon::SourceId source_id_, bool buf_owner, snort::Flow* flow_,
-        const HttpParaList* params_)
-        : HttpMsgSection(buffer, buf_size, session_data_, source_id_, buf_owner, flow_, params_)
-        { }
+        const HttpParaList* params_);
     ~HttpMsgHeadShared() override;
     // Get the next item in a comma-separated header value and convert it to an enum value
     static int32_t get_next_code(const Field& field, int32_t& offset, const StrCode table[]);
@@ -130,6 +128,8 @@ private:
     Field content_disposition_filename;
     uint64_t file_cache_index = 0;
     bool file_cache_index_computed = false;
+
+    bool own_msg_buffer;
 };
 
 #endif
diff --git a/src/service_inspectors/http_inspect/http_msg_start.cc b/src/service_inspectors/http_inspect/http_msg_start.cc
index 276a6b781..06f9621c0 100644
--- a/src/service_inspectors/http_inspect/http_msg_start.cc
+++ b/src/service_inspectors/http_inspect/http_msg_start.cc
@@ -27,6 +27,21 @@
 
 using namespace HttpEnums;
 
+HttpMsgStart::HttpMsgStart(const uint8_t* buffer, const uint16_t buf_size, HttpFlowData* session_data_,
+    HttpCommon::SourceId source_id_, bool buf_owner, snort::Flow* flow_,
+    const HttpParaList* params_) : HttpMsgSection(buffer, buf_size, session_data_, source_id_,
+    buf_owner, flow_, params_), own_msg_buffer(buf_owner)
+{
+    if (own_msg_buffer)
+        session_data->update_allocations(msg_text.length());
+}
+
+HttpMsgStart::~HttpMsgStart()
+{
+    if (own_msg_buffer)
+        session_data->update_deallocations(msg_text.length());
+}
+
 void HttpMsgStart::analyze()
 {
     start_line.set(msg_text);
diff --git a/src/service_inspectors/http_inspect/http_msg_start.h b/src/service_inspectors/http_inspect/http_msg_start.h
index 124b74f0e..a265dec7f 100644
--- a/src/service_inspectors/http_inspect/http_msg_start.h
+++ b/src/service_inspectors/http_inspect/http_msg_start.h
@@ -38,14 +38,14 @@ public:
 protected:
     HttpMsgStart(const uint8_t* buffer, const uint16_t buf_size, HttpFlowData* session_data_,
         HttpCommon::SourceId source_id_, bool buf_owner, snort::Flow* flow_,
-        const HttpParaList* params_)
-        : HttpMsgSection(buffer, buf_size, session_data_, source_id_, buf_owner, flow_, params_)
-        { }
+        const HttpParaList* params_);
+    ~HttpMsgStart() override;
     virtual void parse_start_line() = 0;
     void derive_version_id();
 
     Field start_line;
     Field version;
+    bool own_msg_buffer;
 };
 
 #endif
diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
index 478dab8bf..a0c46cc78 100644
--- a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
+++ b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
@@ -386,8 +386,9 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total,
         assert(session_data->section_offset[source_id] == 0);
         memcpy(buffer, partial_buffer, partial_buffer_length);
         session_data->section_offset[source_id] = partial_buffer_length;
-        partial_buffer_length = 0;
         delete[] partial_buffer;
+        session_data->update_deallocations(partial_buffer_length);
+        partial_buffer_length = 0;
         partial_buffer = nullptr;
     }
 
@@ -420,6 +421,7 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total,
             memcpy(partial_buffer, buffer, buf_size);
             partial_buffer_length = buf_size;
             partial_raw_bytes += total;
+            session_data->update_allocations(partial_buffer_length);
         }
         else
             partial_raw_bytes = 0;
diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc
index 683e82c4d..1ba2d16e1 100755
--- a/src/service_inspectors/http_inspect/http_tables.cc
+++ b/src/service_inspectors/http_inspect/http_tables.cc
@@ -438,6 +438,8 @@ const PegInfo HttpModule::peg_names[PEG_COUNT_MAX+1] =
     { CountType::SUM, "parameters", "HTTP parameters inspected" },
     { CountType::SUM, "connect_tunnel_cutovers", "CONNECT tunnel flow cutovers to wizard" },
     { CountType::SUM, "ssl_srch_abandoned_early", "total SSL search abandoned too soon" },
+    { CountType::SUM, "pipelined_flows", "total HTTP connections containing pipelined requests" },
+    { CountType::SUM, "pipelined_requests", "total requests placed in a pipeline" },
     { CountType::END, nullptr, nullptr }
 };
 
diff --git a/src/service_inspectors/http_inspect/http_transaction.cc b/src/service_inspectors/http_inspect/http_transaction.cc
index e31e6f4a7..dec0f2f55 100644
--- a/src/service_inspectors/http_inspect/http_transaction.cc
+++ b/src/service_inspectors/http_inspect/http_transaction.cc
@@ -36,6 +36,10 @@ using namespace HttpCommon;
 using namespace HttpEnums;
 using namespace snort;
 
+const uint16_t HttpTransaction::transaction_memory_usage_estimate = sizeof(HttpTransaction) +
+    sizeof(HttpMsgRequest) + sizeof(HttpMsgStatus) + (2 * sizeof(HttpMsgHeader)) + sizeof(HttpUri)
+    + (2 * sizeof(HttpInfractions)) + small_things;
+
 static void delete_section_list(HttpMsgSection* section_list)
 {
     while (section_list != nullptr)
@@ -46,6 +50,13 @@ static void delete_section_list(HttpMsgSection* section_list)
     }
 }
 
+HttpTransaction::HttpTransaction(HttpFlowData* session_data_): session_data(session_data_)
+{
+    infractions[0] = nullptr;
+    infractions[1] = nullptr;
+    session_data->update_allocations(transaction_memory_usage_estimate);
+}
+
 HttpTransaction::~HttpTransaction()
 {
     delete request;
@@ -58,6 +69,7 @@ HttpTransaction::~HttpTransaction()
     }
     delete_section_list(body_list);
     delete_section_list(discard_list);
+    session_data->update_deallocations(transaction_memory_usage_estimate);
 }
 
 HttpTransaction* HttpTransaction::attach_my_transaction(HttpFlowData* session_data, SourceId
diff --git a/src/service_inspectors/http_inspect/http_transaction.h b/src/service_inspectors/http_inspect/http_transaction.h
index aa9fc19aa..df9457b86 100644
--- a/src/service_inspectors/http_inspect/http_transaction.h
+++ b/src/service_inspectors/http_inspect/http_transaction.h
@@ -69,11 +69,7 @@ public:
     HttpTransaction* next = nullptr;
 
 private:
-    HttpTransaction(HttpFlowData* session_data_) : session_data(session_data_)
-    {
-        infractions[0] = nullptr;
-        infractions[1] = nullptr;
-    }
+    HttpTransaction(HttpFlowData* session_data_);
     void discard_section(HttpMsgSection* section);
 
     HttpFlowData* const session_data;
@@ -96,6 +92,10 @@ private:
     // transaction in the fairly rare case where the request and response are received in
     // parallel.
     bool shared_ownership = false;
+
+    // Estimates of how much memory http_inspect uses to process a transaction
+    static const uint16_t small_things = 400; // minor memory costs not otherwise accounted for
+    static const uint16_t transaction_memory_usage_estimate;
 };
 
 #endif
diff --git a/src/service_inspectors/http_inspect/test/http_transaction_test.cc b/src/service_inspectors/http_inspect/test/http_transaction_test.cc
index 85ae1e600..f6dd73685 100644
--- a/src/service_inspectors/http_inspect/test/http_transaction_test.cc
+++ b/src/service_inspectors/http_inspect/test/http_transaction_test.cc
@@ -46,6 +46,8 @@ FlowData::~FlowData() = default;
 int DetectionEngine::queue_event(unsigned int, unsigned int, Actions::Type) { return 0; }
 fd_status_t File_Decomp_StopFree(fd_session_t*) { return File_Decomp_OK; }
 uint32_t str_to_hash(const uint8_t *, size_t) { return 0; }
+void FlowData::update_allocations(unsigned long) {}
+void FlowData::update_deallocations(unsigned long) {}
 }
 
 THREAD_LOCAL PegCount HttpModule::peg_counts[PEG_COUNT_MAX] = { };