From: Victor Julien Date: Sun, 24 Sep 2023 04:51:33 +0000 (+0200) Subject: detect/content-inspect: localize recursion counting X-Git-Tag: suricata-8.0.0-beta1~1953 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4cce7ba48b5055eb48ec641ca8e980eb3d7e4b62;p=thirdparty%2Fsuricata.git detect/content-inspect: localize recursion counting 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. --- diff --git a/src/detect-engine-content-inspection.c b/src/detect-engine-content-inspection.c index 3a3b786e46..868a26d3ce 100644 --- a/src/detect-engine-content-inspection.c +++ b/src/detect-engine-content-inspection.c @@ -66,6 +66,17 @@ #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 @@ -95,18 +105,17 @@ * \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 diff --git a/src/detect-tls-sni.c b/src/detect-tls-sni.c index 69b066e8e9..6ac644f1de 100644 --- a/src/detect-tls-sni.c +++ b/src/detect-tls-sni.c @@ -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 diff --git a/src/detect.h b/src/detect.h index 3861b603d8..9230f501d6 100644 --- a/src/detect.h +++ b/src/detect.h @@ -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; diff --git a/src/tests/detect-engine-content-inspection.c b/src/tests/detect-engine-content-inspection.c index ee1b605f2c..65a4f578b8 100644 --- a/src/tests/detect-engine-content-inspection.c +++ b/src/tests/detect-engine-content-inspection.c @@ -29,33 +29,34 @@ #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