]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
streaming/buffer: fix sliding region into next
authorVictor Julien <vjulien@oisf.net>
Tue, 6 Jun 2023 10:17:16 +0000 (12:17 +0200)
committerVictor Julien <vjulien@oisf.net>
Wed, 7 Jun 2023 19:45:30 +0000 (21:45 +0200)
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.

src/util-streaming-buffer.c

index 2b72e394b2eaead906857caaffed9d4294fbeb2c..39147ebc1e94301d37cab36730dda6cc2ef621f5 100644 (file)
@@ -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;
         }
     }