]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect/content-inspect: localize recursion counting
authorVictor Julien <vjulien@oisf.net>
Sun, 24 Sep 2023 04:51:33 +0000 (06:51 +0200)
committerVictor Julien <victor@inliniac.net>
Thu, 7 Dec 2023 08:56:59 +0000 (09:56 +0100)
Use stack local var instead of DetectEngineThreadCtx member. Instead
setup a stack local struct that both counts and holds the limit. Make sure
the limit is a const so we can avoid rereading it.

This is part of an effort to reduce the size of the DetectEngineThreadCtx
structure and reduce the number of memory writes to it. Additionally, it
is part of an effect to reduce the number of places where detection
tracks various forms of state.

src/detect-engine-content-inspection.c
src/detect-tls-sni.c
src/detect.h
src/tests/detect-engine-content-inspection.c

index 3a3b786e467034ca3d0f1b716a99fd3a00584fd8..868a26d3ce12ceb784132ba9cc294e947fe70959 100644 (file)
 #include "util-lua.h"
 #endif
 
+#ifdef UNITTESTS
+thread_local uint32_t ut_inspection_recursion_counter = 0;
+#endif
+
+struct DetectEngineContentInspectionCtx {
+    struct {
+        uint32_t count;
+        const uint32_t limit;
+    } recursion;
+};
+
 /**
  * \brief Run the actual payload match functions
  *
@@ -74,7 +85,6 @@
  * For accounting the last match in relative matching the
  * det_ctx->buffer_offset int is used.
  *
- * \param de_ctx          Detection engine context
  * \param det_ctx         Detection engine thread context
  * \param s               Signature to inspect
  * \param sm              SigMatch to inspect
  *  \retval 0 no match
  *  \retval 1 match
  */
-static int DetectEngineContentInspectionInternal(DetectEngineCtx *de_ctx,
-        DetectEngineThreadCtx *det_ctx, const Signature *s, const SigMatchData *smd, Packet *p,
-        Flow *f, const uint8_t *buffer, const uint32_t buffer_len,
+static int DetectEngineContentInspectionInternal(DetectEngineThreadCtx *det_ctx,
+        struct DetectEngineContentInspectionCtx *ctx, const Signature *s, const SigMatchData *smd,
+        Packet *p, Flow *f, const uint8_t *buffer, const uint32_t buffer_len,
         const uint32_t stream_start_offset, const uint8_t flags,
         const enum DetectContentInspectionType inspection_mode)
 {
     SCEnter();
     KEYWORD_PROFILING_START;
 
-    det_ctx->inspection_recursion_counter++;
-
-    if (unlikely(det_ctx->inspection_recursion_counter == de_ctx->inspection_recursion_limit)) {
+    ctx->recursion.count++;
+    if (unlikely(ctx->recursion.count == ctx->recursion.limit)) {
         KEYWORD_PROFILING_END(det_ctx, smd->type, 0);
         SCReturnInt(-1);
     }
@@ -349,9 +358,8 @@ static int DetectEngineContentInspectionInternal(DetectEngineCtx *de_ctx,
                         /* see if the next buffer keywords match. If not, we will
                          * search for another occurrence of this content and see
                          * if the others match then until we run out of matches */
-                        int r = DetectEngineContentInspectionInternal(de_ctx, det_ctx, s, smd + 1,
-                                p, f, buffer, buffer_len, stream_start_offset, flags,
-                                inspection_mode);
+                        int r = DetectEngineContentInspectionInternal(det_ctx, ctx, s, smd + 1, p,
+                                f, buffer, buffer_len, stream_start_offset, flags, inspection_mode);
                         if (r == 1) {
                             SCReturnInt(1);
                         } else if (r == -1) {
@@ -448,7 +456,7 @@ static int DetectEngineContentInspectionInternal(DetectEngineCtx *de_ctx,
             /* see if the next payload keywords match. If not, we will
              * search for another occurrence of this pcre and see
              * if the others match, until we run out of matches */
-            r = DetectEngineContentInspectionInternal(de_ctx, det_ctx, s, smd + 1, p, f, buffer,
+            r = DetectEngineContentInspectionInternal(det_ctx, ctx, s, smd + 1, p, f, buffer,
                     buffer_len, stream_start_offset, flags, inspection_mode);
             if (r == 1) {
                 SCReturnInt(1);
@@ -654,7 +662,7 @@ static int DetectEngineContentInspectionInternal(DetectEngineCtx *de_ctx,
             if (s->sm_arrays[DETECT_SM_LIST_BASE64_DATA] != NULL) {
                 if (det_ctx->base64_decoded_len) {
                     KEYWORD_PROFILING_END(det_ctx, smd->type, 1);
-                    int r = DetectEngineContentInspectionInternal(de_ctx, det_ctx, s,
+                    int r = DetectEngineContentInspectionInternal(det_ctx, ctx, s,
                             s->sm_arrays[DETECT_SM_LIST_BASE64_DATA], NULL, f,
                             det_ctx->base64_decoded, det_ctx->base64_decoded_len, 0,
                             DETECT_CI_FLAGS_SINGLE, DETECT_ENGINE_CONTENT_INSPECTION_MODE_STATE);
@@ -692,7 +700,7 @@ match:
      * the buffer portion of the signature matched. */
     if (!smd->is_last) {
         KEYWORD_PROFILING_END(det_ctx, smd->type, 1);
-        int r = DetectEngineContentInspectionInternal(de_ctx, det_ctx, s, smd + 1, p, f, buffer,
+        int r = DetectEngineContentInspectionInternal(det_ctx, ctx, s, smd + 1, p, f, buffer,
                 buffer_len, stream_start_offset, flags, inspection_mode);
         SCReturnInt(r);
     }
@@ -710,11 +718,15 @@ bool DetectEngineContentInspection(DetectEngineCtx *de_ctx, DetectEngineThreadCt
         const uint32_t buffer_len, const uint32_t stream_start_offset, const uint8_t flags,
         const enum DetectContentInspectionType inspection_mode)
 {
+    struct DetectEngineContentInspectionCtx ctx = { .recursion.count = 0,
+        .recursion.limit = de_ctx->inspection_recursion_limit };
     det_ctx->buffer_offset = 0;
-    det_ctx->inspection_recursion_counter = 0;
 
-    int r = DetectEngineContentInspectionInternal(de_ctx, det_ctx, s, smd, p, f, buffer, buffer_len,
+    int r = DetectEngineContentInspectionInternal(det_ctx, &ctx, s, smd, p, f, buffer, buffer_len,
             stream_start_offset, flags, inspection_mode);
+#ifdef UNITTESTS
+    ut_inspection_recursion_counter = ctx.recursion.count;
+#endif
     if (r == 1)
         return true;
     else
@@ -729,11 +741,16 @@ bool DetectEngineContentInspectionBuffer(DetectEngineCtx *de_ctx, DetectEngineTh
         const Signature *s, const SigMatchData *smd, Packet *p, Flow *f, const InspectionBuffer *b,
         const enum DetectContentInspectionType inspection_mode)
 {
+    struct DetectEngineContentInspectionCtx ctx = { .recursion.count = 0,
+        .recursion.limit = de_ctx->inspection_recursion_limit };
+
     det_ctx->buffer_offset = 0;
-    det_ctx->inspection_recursion_counter = 0;
 
-    int r = DetectEngineContentInspectionInternal(de_ctx, det_ctx, s, smd, p, f, b->inspect,
+    int r = DetectEngineContentInspectionInternal(det_ctx, &ctx, s, smd, p, f, b->inspect,
             b->inspect_len, b->inspect_offset, b->flags, inspection_mode);
+#ifdef UNITTESTS
+    ut_inspection_recursion_counter = ctx.recursion.count;
+#endif
     if (r == 1)
         return true;
     else
index 69b066e8e979faaae90aa34a6b5fb95137826841..6ac644f1de3abdb9e728ed43abfe2e68fb2072df 100644 (file)
@@ -1,4 +1,4 @@
-/* Copyright (C) 2007-2016 Open Information Security Foundation
+/* Copyright (C) 2007-2023 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
index 3861b603d801d5675852f7f2e8f284591a3cfbfb..9230f501d697d846b33e5db4e50fe0661883d89c 100644 (file)
@@ -1159,9 +1159,6 @@ typedef struct DetectEngineThreadCtx_ {
 
     SC_ATOMIC_DECLARE(int, so_far_used_by_detect);
 
-    /* holds the current recursion depth on content inspection */
-    int inspection_recursion_counter;
-
     /** array of signature pointers we're going to inspect in the detection
      *  loop. */
     Signature **match_array;
index ee1b605f2c0d7254ab62dbed025963f307570b3b..65a4f578b824bafe8f2c177b31cb3f4358c6b2e3 100644 (file)
 #include "../detect.h"
 #include "detect-engine-build.h"
 
+extern thread_local uint32_t ut_inspection_recursion_counter;
+
 #define TEST_HEADER                                     \
     ThreadVars tv;                                      \
     memset(&tv, 0, sizeof(tv));                         \
     Flow f;                                             \
     memset(&f, 0, sizeof(f));
 
-#define TEST_RUN(buf, buflen, sig, match, steps)                                            \
-{                                                                                           \
-    DetectEngineCtx *de_ctx = DetectEngineCtxInit();                                        \
-    FAIL_IF_NULL(de_ctx);                                                                   \
-    DetectEngineThreadCtx *det_ctx = NULL;                                                  \
-    char rule[2048];                                                                        \
-    snprintf(rule, sizeof(rule), "alert tcp any any -> any any (%s sid:1; rev:1;)", (sig)); \
-    Signature *s = DetectEngineAppendSig(de_ctx, rule);                                     \
-    FAIL_IF_NULL(s);                                                                        \
-    SigGroupBuild(de_ctx);                                                                  \
-    DetectEngineThreadCtxInit(&tv, (void *)de_ctx, (void *)&det_ctx);                       \
-    FAIL_IF_NULL(det_ctx);                                                                  \
-    int r = DetectEngineContentInspection(de_ctx, det_ctx,                                  \
-                s, s->sm_arrays[DETECT_SM_LIST_PMATCH], NULL, &f,                           \
-                (uint8_t *)(buf), (buflen), 0, DETECT_CI_FLAGS_SINGLE,                      \
-                DETECT_ENGINE_CONTENT_INSPECTION_MODE_PAYLOAD);                             \
-    FAIL_IF_NOT(r == (match));                                                              \
-    FAIL_IF_NOT(det_ctx->inspection_recursion_counter == (steps));                          \
-    DetectEngineThreadCtxDeinit(&tv, det_ctx);                                              \
-    DetectEngineCtxFree(de_ctx);                                                            \
-}
+#define TEST_RUN(buf, buflen, sig, match, steps)                                                   \
+    {                                                                                              \
+        DetectEngineCtx *de_ctx = DetectEngineCtxInit();                                           \
+        FAIL_IF_NULL(de_ctx);                                                                      \
+        DetectEngineThreadCtx *det_ctx = NULL;                                                     \
+        char rule[2048];                                                                           \
+        snprintf(rule, sizeof(rule), "alert tcp any any -> any any (%s sid:1; rev:1;)", (sig));    \
+        Signature *s = DetectEngineAppendSig(de_ctx, rule);                                        \
+        FAIL_IF_NULL(s);                                                                           \
+        SigGroupBuild(de_ctx);                                                                     \
+        DetectEngineThreadCtxInit(&tv, (void *)de_ctx, (void *)&det_ctx);                          \
+        FAIL_IF_NULL(det_ctx);                                                                     \
+        int r = DetectEngineContentInspection(de_ctx, det_ctx, s,                                  \
+                s->sm_arrays[DETECT_SM_LIST_PMATCH], NULL, &f, (uint8_t *)(buf), (buflen), 0,      \
+                DETECT_CI_FLAGS_SINGLE, DETECT_ENGINE_CONTENT_INSPECTION_MODE_PAYLOAD);            \
+        FAIL_IF_NOT(r == (match));                                                                 \
+        FAIL_IF_NOT(ut_inspection_recursion_counter == (steps));                                   \
+        DetectEngineThreadCtxDeinit(&tv, det_ctx);                                                 \
+        DetectEngineCtxFree(de_ctx);                                                               \
+    }
 #define TEST_FOOTER     \
     PASS