From: Victor Julien <vjulien@oisf.net>
Date: Tue, 6 Jun 2023 10:17:16 +0000 (+0200)
Subject: streaming/buffer: fix sliding region into next
X-Git-Tag: suricata-7.0.0-rc2~65
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e69583da54d957bb0f96d853272c9ba83d56b1c8;p=thirdparty%2Fsuricata.git

streaming/buffer: fix sliding region into next

When sliding a region it could start to overlap with the next region.
This case wasn't handled, causing validation checks to trigger.

This patch adds support for this, where largest region will be expanded
to fit both region and both regions will be consolidated into it.

Bug: #6066.
---

diff --git a/src/util-streaming-buffer.c b/src/util-streaming-buffer.c
index 2b72e394b2..39147ebc1e 100644
--- a/src/util-streaming-buffer.c
+++ b/src/util-streaming-buffer.c
@@ -845,19 +845,101 @@ static inline void StreamingBufferSlideToOffsetWithRegions(
         if (s > 0) {
             const uint32_t new_data_size = to_shift->buf_size - s;
             const uint32_t new_mem_size = ToNextMultipleOf(new_data_size, cfg->buf_size);
-            SCLogDebug("s %u new_data_size %u", s, new_data_size);
-            memmove(to_shift->buf, to_shift->buf + s, new_data_size);
-            /* shrink memory region. If this fails we keep the old */
-            void *ptr = REALLOC(cfg, to_shift->buf, to_shift->buf_size, new_mem_size);
-            if (ptr != NULL) {
-                to_shift->buf = ptr;
-                to_shift->buf_size = new_mem_size;
+
+            /* see if after the slide we'd overlap with the next region. If so, we need
+             * to consolidate them into one. */
+            if (to_shift->next && slide_offset + new_mem_size >= to_shift->next->stream_offset) {
+                StreamingBufferRegion *start = to_shift;
+                StreamingBufferRegion *next = start->next;
+                const uint64_t next_re = next->stream_offset + next->buf_size;
+                const uint32_t mem_size = ToNextMultipleOf(next_re - slide_offset, cfg->buf_size);
+
+                /* using next as the new main */
+                if (start->buf_size < next->buf_size) {
+                    SCLogDebug("replace main with the next bigger region");
+
+                    const uint32_t next_data_offset = next->stream_offset - slide_offset;
+                    const uint32_t prev_buf_size = next->buf_size;
+
+                    /* expand "next" to include relevant part of "start" */
+                    if (GrowRegionToSize(sb, cfg, next, mem_size) != 0)
+                        return; // TODO what to do?
+                    SCLogDebug("region now sized %u", mem_size);
+
+                    // slide "next":
+                    // pre-grow: [nextnextnext]
+                    // post-grow [nextnextnextXXX]
+                    // post-move [XXXnextnextnext]
+                    memmove(next->buf + next_data_offset, next->buf, prev_buf_size);
+
+                    // move portion of "start" into "next"
+                    //
+                    // start: [ooooookkkkk] (o: old, k: keep)
+                    // pre-next     [xxxxxnextnextnext]
+                    // post-next    [kkkkknextnextnext]
+                    const uint32_t start_data_offset = slide_offset - start->stream_offset;
+                    BUG_ON(start_data_offset > start->buf_size);
+                    const uint32_t start_data_size = start->buf_size - start_data_offset;
+                    memcpy(next->buf, start->buf + start_data_offset, start_data_size);
+
+                    // free "start"s buffer, we will use the one from "next"
+                    FREE(cfg, start->buf, start->buf_size);
+
+                    // update "main" to use "next"
+                    start->stream_offset = slide_offset;
+                    start->buf = next->buf;
+                    start->buf_size = next->buf_size;
+                    start->next = next->next;
+
+                    // free "next"
+                    FREE(cfg, next, sizeof(*next));
+                    sb->regions--;
+                    BUG_ON(sb->regions == 0);
+                } else {
+                    /* using "main", expand to include "next" */
+                    if (GrowRegionToSize(sb, cfg, start, mem_size) != 0)
+                        return; // TODO what to do?
+                    SCLogDebug("start->buf now size %u", mem_size);
+
+                    // slide "start"
+                    // pre:     [xxxxxxxAAA]
+                    // post:    [AAAxxxxxxx]
+                    SCLogDebug("s %u new_data_size %u", s, new_data_size);
+                    memmove(start->buf, start->buf + s, new_data_size);
+
+                    // copy in "next"
+                    // pre:     [AAAxxxxxxx]
+                    //             [BBBBBBB]
+                    // post:    [AAABBBBBBB]
+                    SCLogDebug("copy next->buf %p/%u to start->buf offset %u", next->buf,
+                            next->buf_size, new_data_size);
+                    memcpy(start->buf + new_data_size, next->buf, next->buf_size);
+
+                    start->stream_offset = slide_offset;
+                    start->next = next->next;
+
+                    // free "next"
+                    FREE(cfg, next->buf, next->buf_size);
+                    FREE(cfg, next, sizeof(*next));
+                    sb->regions--;
+                    BUG_ON(sb->regions == 0);
+                }
+            } else {
+                SCLogDebug("s %u new_data_size %u", s, new_data_size);
+                memmove(to_shift->buf, to_shift->buf + s, new_data_size);
+                /* shrink memory region. If this fails we keep the old */
+                void *ptr = REALLOC(cfg, to_shift->buf, to_shift->buf_size, new_mem_size);
+                if (ptr != NULL) {
+                    to_shift->buf = ptr;
+                    to_shift->buf_size = new_mem_size;
+                    SCLogDebug("new buf_size %u", to_shift->buf_size);
+                }
+                if (s < to_shift->buf_offset)
+                    to_shift->buf_offset -= s;
+                else
+                    to_shift->buf_offset = 0;
+                to_shift->stream_offset = slide_offset;
             }
-            if (s < to_shift->buf_offset)
-                to_shift->buf_offset -= s;
-            else
-                to_shift->buf_offset = 0;
-            to_shift->stream_offset = slide_offset;
         }
     }