From e96464308819afb44d3b95d97f10ab84b6e0e979 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Sun, 28 Feb 2021 09:39:16 +0100 Subject: [PATCH] detect/state: fix reset bug Fix issue where after a reset the now empty list elements are not reused and the values may not be valid for the current detect engine anymore. Introduce a 'current' (cur) pointer that points to the store element currently being filled. This way existing stores will be reused. If 'cur' is NULL and 'head' is not NULL it means we need to use 'tail' to append a new store. --- src/detect-engine-state.c | 58 ++++++++++++++++++++++++++++++--------- src/detect-engine-state.h | 5 ++-- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/detect-engine-state.c b/src/detect-engine-state.c index fa9e617a5c..63be2a6147 100644 --- a/src/detect-engine-state.c +++ b/src/detect-engine-state.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2013 Open Information Security Foundation +/* Copyright (C) 2007-2021 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 @@ -131,28 +131,32 @@ static void DeStateSignatureAppend(DetectEngineState *state, BUG_ON(DeStateSearchState(state, direction, s->num)); #endif DeStateStore *store = dir_state->tail; - if (store == NULL) { store = DeStateStoreAlloc(); dir_state->head = store; + dir_state->cur = store; dir_state->tail = store; + } else if (dir_state->cur) { + store = dir_state->cur; } else { - SCLogDebug("dir_state->cnt %u mod chunksize %u", dir_state->cnt, - dir_state->cnt % DE_STATE_CHUNK_SIZE); - if (dir_state->cnt && dir_state->cnt % DE_STATE_CHUNK_SIZE == 0) { - store = DeStateStoreAlloc(); - if (store != NULL) { - dir_state->tail->next = store; - dir_state->tail = store; - } + store = DeStateStoreAlloc(); + if (store != NULL) { + dir_state->tail->next = store; + dir_state->tail = store; + dir_state->cur = store; } } if (store == NULL) SCReturn; - SigIntId idx = dir_state->cnt++ % DE_STATE_CHUNK_SIZE; + SigIntId idx = dir_state->cnt % DE_STATE_CHUNK_SIZE; store->store[idx].sid = s->num; store->store[idx].flags = inspect_flags; + dir_state->cnt++; + /* if current chunk is full, progress cur */ + if (dir_state->cnt % DE_STATE_CHUNK_SIZE == 0) { + dir_state->cur = dir_state->cur->next; + } SCReturn; } @@ -261,10 +265,14 @@ static inline void ResetTxState(DetectEngineState *s) s->dir_state[0].cnt = 0; s->dir_state[0].filestore_cnt = 0; s->dir_state[0].flags = 0; + /* reset 'cur' back to the list head */ + s->dir_state[0].cur = s->dir_state[0].head; s->dir_state[1].cnt = 0; s->dir_state[1].filestore_cnt = 0; s->dir_state[1].flags = 0; + /* reset 'cur' back to the list head */ + s->dir_state[1].cur = s->dir_state[1].head; } } @@ -313,14 +321,14 @@ static int DeStateTest01(void) static int DeStateTest02(void) { + uint8_t direction = STREAM_TOSERVER; DetectEngineState *state = DetectEngineStateAlloc(); FAIL_IF_NULL(state); + FAIL_IF_NOT_NULL(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].head); Signature s; memset(&s, 0x00, sizeof(s)); - uint8_t direction = STREAM_TOSERVER; - s.num = 0; DeStateSignatureAppend(state, &s, 0, direction); s.num = 11; @@ -349,10 +357,23 @@ static int DeStateTest02(void) DeStateSignatureAppend(state, &s, 0, direction); s.num = 133; DeStateSignatureAppend(state, &s, 0, direction); + FAIL_IF_NOT(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].head == + state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].cur); + s.num = 144; DeStateSignatureAppend(state, &s, 0, direction); + + FAIL_IF(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].head->store[14].sid != 144); + FAIL_IF(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].head == + state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].cur); + FAIL_IF_NOT(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].cur == NULL); + s.num = 155; DeStateSignatureAppend(state, &s, 0, direction); + + FAIL_IF_NOT(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].tail == + state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].cur); + s.num = 166; DeStateSignatureAppend(state, &s, 0, direction); @@ -365,6 +386,10 @@ static int DeStateTest02(void) ResetTxState(state); + FAIL_IF(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].head == NULL); + FAIL_IF_NOT(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].head == + state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].cur); + s.num = 0; DeStateSignatureAppend(state, &s, 0, direction); s.num = 11; @@ -393,8 +418,15 @@ static int DeStateTest02(void) DeStateSignatureAppend(state, &s, 0, direction); s.num = 133; DeStateSignatureAppend(state, &s, 0, direction); + FAIL_IF_NOT(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].head == + state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].cur); s.num = 144; DeStateSignatureAppend(state, &s, 0, direction); + FAIL_IF(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].head->store[14].sid != 144); + FAIL_IF(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].head == + state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].cur); + FAIL_IF_NOT(state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].tail == + state->dir_state[direction & STREAM_TOSERVER ? 0 : 1].cur); s.num = 155; DeStateSignatureAppend(state, &s, 0, direction); s.num = 166; diff --git a/src/detect-engine-state.h b/src/detect-engine-state.h index 4491105619..3363e2dd39 100644 --- a/src/detect-engine-state.h +++ b/src/detect-engine-state.h @@ -81,8 +81,9 @@ typedef struct DeStateStore_ { } DeStateStore; typedef struct DetectEngineStateDirection_ { - DeStateStore *head; - DeStateStore *tail; + DeStateStore *head; /**< head of the list */ + DeStateStore *cur; /**< current active store */ + DeStateStore *tail; /**< tail of the list */ SigIntId cnt; uint16_t filestore_cnt; uint8_t flags;