From f80ebd5a30b02db5915f749f0c067c7adefbbe76 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 7 Nov 2024 17:49:45 +0100 Subject: [PATCH] detect/transforms: write directly in inspect buffer instead of writing to a temporary buffer and then copying, to save the cost of copying. Ticket: 7229 Not a cherry-pick as we do not put the transforms in rust, but just do this optimization in C --- src/detect-engine.c | 23 +++++++++++++++++++-- src/detect-engine.h | 3 ++- src/detect-transform-casechange.c | 14 +++++++++---- src/detect-transform-compress-whitespace.c | 8 +++++-- src/detect-transform-dotprefix.c | 8 +++++-- src/detect-transform-header-lowercase.c | 7 +++++-- src/detect-transform-strip-pseudo-headers.c | 7 +++++-- src/detect-transform-strip-whitespace.c | 8 +++++-- src/detect-transform-urldecode.c | 8 +++++-- src/detect-transform-xor.c | 7 +++++-- 10 files changed, 72 insertions(+), 21 deletions(-) diff --git a/src/detect-engine.c b/src/detect-engine.c index 9cfb222dd421..f1c47b0fa356 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -1653,11 +1653,13 @@ void InspectionBufferFree(InspectionBuffer *buffer) /** * \brief make sure that the buffer has at least 'min_size' bytes * Expand the buffer if necessary + * + * \retval pointer to inner buffer to use, or NULL if realloc failed */ -void InspectionBufferCheckAndExpand(InspectionBuffer *buffer, uint32_t min_size) +uint8_t *InspectionBufferCheckAndExpand(InspectionBuffer *buffer, uint32_t min_size) { if (likely(buffer->size >= min_size)) - return; + return buffer->buf; uint32_t new_size = (buffer->size == 0) ? 4096 : buffer->size; while (new_size < min_size) { @@ -1668,7 +1670,24 @@ void InspectionBufferCheckAndExpand(InspectionBuffer *buffer, uint32_t min_size) if (ptr != NULL) { buffer->buf = ptr; buffer->size = new_size; + } else { + return NULL; } + return buffer->buf; +} + +/** + * \brief set inspect length of inspect buffer + * The inspect buffer may have been overallocated (by strip_whitespace for example) + * so, this sets the final length + */ +void InspectionBufferTruncate(InspectionBuffer *buffer, uint32_t buf_len) +{ + DEBUG_VALIDATE_BUG_ON(buffer->buf == NULL); + DEBUG_VALIDATE_BUG_ON(buf_len > buffer->size); + buffer->inspect = buffer->buf; + buffer->inspect_len = buf_len; + buffer->initialized = true; } void InspectionBufferCopy(InspectionBuffer *buffer, uint8_t *buf, uint32_t buf_len) diff --git a/src/detect-engine.h b/src/detect-engine.h index a1732b16a993..006aff972aa5 100644 --- a/src/detect-engine.h +++ b/src/detect-engine.h @@ -31,7 +31,8 @@ void InspectionBufferInit(InspectionBuffer *buffer, uint32_t initial_size); void InspectionBufferSetup(DetectEngineThreadCtx *det_ctx, const int list_id, InspectionBuffer *buffer, const uint8_t *data, const uint32_t data_len); void InspectionBufferFree(InspectionBuffer *buffer); -void InspectionBufferCheckAndExpand(InspectionBuffer *buffer, uint32_t min_size); +uint8_t *InspectionBufferCheckAndExpand(InspectionBuffer *buffer, uint32_t min_size); +void InspectionBufferTruncate(InspectionBuffer *buffer, uint32_t buf_len); void InspectionBufferCopy(InspectionBuffer *buffer, uint8_t *buf, uint32_t buf_len); void InspectionBufferApplyTransforms(InspectionBuffer *buffer, const DetectEngineTransforms *transforms); diff --git a/src/detect-transform-casechange.c b/src/detect-transform-casechange.c index 851030828ced..f22667033612 100644 --- a/src/detect-transform-casechange.c +++ b/src/detect-transform-casechange.c @@ -62,12 +62,15 @@ static void DetectTransformToLower(InspectionBuffer *buffer, void *options) return; } - uint8_t output[input_len]; + uint8_t *output = InspectionBufferCheckAndExpand(buffer, input_len); + if (output == NULL) { + return; + } for (uint32_t i = 0; i < input_len; i++) { output[i] = u8_tolower(input[i]); } - InspectionBufferCopy(buffer, output, input_len); + InspectionBufferTruncate(buffer, input_len); } /** * \internal @@ -102,12 +105,15 @@ static void DetectTransformToUpper(InspectionBuffer *buffer, void *options) return; } - uint8_t output[input_len]; + uint8_t *output = InspectionBufferCheckAndExpand(buffer, input_len); + if (output == NULL) { + return; + } for (uint32_t i = 0; i < input_len; i++) { output[i] = u8_toupper(input[i]); } - InspectionBufferCopy(buffer, output, input_len); + InspectionBufferTruncate(buffer, input_len); } /* diff --git a/src/detect-transform-compress-whitespace.c b/src/detect-transform-compress-whitespace.c index 5cbf0fd896f5..cc78c7e62281 100644 --- a/src/detect-transform-compress-whitespace.c +++ b/src/detect-transform-compress-whitespace.c @@ -111,7 +111,11 @@ static void TransformCompressWhitespace(InspectionBuffer *buffer, void *options) return; } - uint8_t output[input_len]; // we can only shrink + // we can only shrink + uint8_t *output = InspectionBufferCheckAndExpand(buffer, input_len); + if (output == NULL) { + return; + } uint8_t *oi = output, *os = output; //PrintRawDataFp(stdout, input, input_len); @@ -132,7 +136,7 @@ static void TransformCompressWhitespace(InspectionBuffer *buffer, void *options) uint32_t output_size = oi - os; //PrintRawDataFp(stdout, output, output_size); - InspectionBufferCopy(buffer, os, output_size); + InspectionBufferTruncate(buffer, output_size); } #ifdef UNITTESTS diff --git a/src/detect-transform-dotprefix.c b/src/detect-transform-dotprefix.c index 52a263372b43..295a149f8941 100644 --- a/src/detect-transform-dotprefix.c +++ b/src/detect-transform-dotprefix.c @@ -110,11 +110,15 @@ static void TransformDotPrefix(InspectionBuffer *buffer, void *options) const size_t input_len = buffer->inspect_len; if (input_len) { - uint8_t output[input_len + 1]; // For the leading '.' + // For the leading '.' + uint8_t *output = InspectionBufferCheckAndExpand(buffer, input_len + 1); + if (output == NULL) { + return; + } output[0] = '.'; memcpy(&output[1], buffer->inspect, input_len); - InspectionBufferCopy(buffer, output, input_len + 1); + InspectionBufferTruncate(buffer, input_len + 1); } } diff --git a/src/detect-transform-header-lowercase.c b/src/detect-transform-header-lowercase.c index 7c776201b308..b04ded3a68ba 100644 --- a/src/detect-transform-header-lowercase.c +++ b/src/detect-transform-header-lowercase.c @@ -53,7 +53,10 @@ static void DetectTransformHeaderLowercase(InspectionBuffer *buffer, void *optio if (input_len == 0) { return; } - uint8_t output[input_len]; + uint8_t *output = InspectionBufferCheckAndExpand(buffer, input_len); + if (output == NULL) { + return; + } // state 0 is header name, 1 is header value int state = 0; @@ -72,7 +75,7 @@ static void DetectTransformHeaderLowercase(InspectionBuffer *buffer, void *optio } } } - InspectionBufferCopy(buffer, output, input_len); + InspectionBufferTruncate(buffer, input_len); } void DetectTransformHeaderLowercaseRegister(void) diff --git a/src/detect-transform-strip-pseudo-headers.c b/src/detect-transform-strip-pseudo-headers.c index 450900d46037..d755d5fa45a5 100644 --- a/src/detect-transform-strip-pseudo-headers.c +++ b/src/detect-transform-strip-pseudo-headers.c @@ -53,7 +53,10 @@ static void DetectTransformStripPseudoHeaders(InspectionBuffer *buffer, void *op if (input_len == 0) { return; } - uint8_t output[input_len]; + uint8_t *output = InspectionBufferCheckAndExpand(buffer, input_len); + if (output == NULL) { + return; + } bool new_line = true; bool pseudo = false; @@ -82,7 +85,7 @@ static void DetectTransformStripPseudoHeaders(InspectionBuffer *buffer, void *op j++; } } - InspectionBufferCopy(buffer, output, j); + InspectionBufferTruncate(buffer, j); } void DetectTransformStripPseudoHeadersRegister(void) diff --git a/src/detect-transform-strip-whitespace.c b/src/detect-transform-strip-whitespace.c index 32fb96f06ea0..60405927a1de 100644 --- a/src/detect-transform-strip-whitespace.c +++ b/src/detect-transform-strip-whitespace.c @@ -106,7 +106,11 @@ static void TransformStripWhitespace(InspectionBuffer *buffer, void *options) if (input_len == 0) { return; } - uint8_t output[input_len]; // we can only shrink + // we can only shrink + uint8_t *output = InspectionBufferCheckAndExpand(buffer, input_len); + if (output == NULL) { + return; + } uint8_t *oi = output, *os = output; //PrintRawDataFp(stdout, input, input_len); @@ -119,7 +123,7 @@ static void TransformStripWhitespace(InspectionBuffer *buffer, void *options) uint32_t output_size = oi - os; //PrintRawDataFp(stdout, output, output_size); - InspectionBufferCopy(buffer, os, output_size); + InspectionBufferTruncate(buffer, output_size); } #ifdef UNITTESTS diff --git a/src/detect-transform-urldecode.c b/src/detect-transform-urldecode.c index 13ef03372f5f..a4e9655ad7e9 100644 --- a/src/detect-transform-urldecode.c +++ b/src/detect-transform-urldecode.c @@ -125,12 +125,16 @@ static void TransformUrlDecode(InspectionBuffer *buffer, void *options) if (input_len == 0) { return; } - uint8_t output[input_len]; // we can only shrink + // we can only shrink + uint8_t *output = InspectionBufferCheckAndExpand(buffer, input_len); + if (output == NULL) { + return; + } changed = BufferUrlDecode(input, input_len, output, &output_size); if (changed) { - InspectionBufferCopy(buffer, output, output_size); + InspectionBufferTruncate(buffer, output_size); } } diff --git a/src/detect-transform-xor.c b/src/detect-transform-xor.c index e42700feb369..18f96df3cf46 100644 --- a/src/detect-transform-xor.c +++ b/src/detect-transform-xor.c @@ -133,12 +133,15 @@ static void DetectTransformXor(InspectionBuffer *buffer, void *options) if (input_len == 0) { return; } - uint8_t output[input_len]; + uint8_t *output = InspectionBufferCheckAndExpand(buffer, input_len); + if (output == NULL) { + return; + } for (uint32_t i = 0; i < input_len; i++) { output[i] = input[i] ^ pxd->key[i % pxd->length]; } - InspectionBufferCopy(buffer, output, input_len); + InspectionBufferTruncate(buffer, input_len); } #ifdef UNITTESTS