The fn DetectFlowvarDataFree checks if the pointer (DetectFlowvarData)
passed to it is valid. So, the checks for NULL in the call sites can
safely be removed.
Issue: 6855: Match sigmatch type field in var and bit structs
Align the size and datatype of type, idx, and next members across:
- FlowVarThreshold
- FlowBit
- FlowVar
- GenericVar
- XBit
- DetectVarList
Note that the FlowVar structure has been intentionally constrained to
match the structure size prior to this commit. To achieve this, the
keylen member was restricted to 8 bits after it was confirmed its value
is checked against a max of 0xff.
Ticket: #6104
And failures should be handled to say that the rule failed to load
Reverts the fix by 299ee6ed55
that was simple, but not complete (memory leak),
to have this bigger API change which simplifies code.
Implement a new design for handling var name id's. The old logic
was aware of detection engine versions and generally didn't work
well for multi-tenancy cases. Other than memory leaks and crashes,
logging of var names worked or failed based on which tenant was
loaded last.
This patch implements a new approach, where there is a global store
of vars and their id's for the lifetime of the program.
Overall Design:
Base Store: "base"
Used during keyword registration. Operates under lock. Base is shared
between all detect engines, detect engine versions and tenants.
Each variable name is ref counted.
During the freeing of a detect engine / tenant, unregistration decreases
the ref cnt.
Base has both a string to id and a id to string hash table. String to
id is used during parsing/registration. id to string during unregistration.
Active Store Pointer (atomic)
The "active" store atomic pointer points to the active lookup store. The call
to `VarNameStoreActivate` will build a new lookup store and hot swap
the pointer.
Ensuring memory safety. During the hot swap, the pointer is replaced, so
any new call to the lookup functions will automatically use the new store.
This leaves the case of any lookup happening concurrently with the pointer
swap. For this case we add the old store to a free list. It gets a timestamp
before which it cannot be freed.
Free List
The free list contains old stores that are waiting to get removed. They
contain a timestamp that is checked before they are freed.
Bug: #6044.
Bug: #6201.
Set flags by default:
-Wmissing-prototypes
-Wmissing-declarations
-Wstrict-prototypes
-Wwrite-strings
-Wcast-align
-Wbad-function-cast
-Wformat-security
-Wno-format-nonliteral
-Wmissing-format-attribute
-funsigned-char
Fix minor compiler warnings for these new flags on gcc and clang.
In preparation of turning input to keyword parsers to const add
options to the common rule parser to enforce and strip double
quotes and parse negation support.
At registration, the keyword can register 3 extra flags:
SIGMATCH_QUOTES_MANDATORY: value to keyword must be quoted
SIGMATCH_QUOTES_OPTIONAL: value to keyword may be quoted
SIGMATCH_HANDLE_NEGATION: leading ! is parsed
In all cases leading spaces are removed. If the 'quote' flags are
set, the quotes are removed from the input as well.
Flowvars were already using a temporary store in the detect thread
ctx.
Use the same facility for pktvars. The reasons are:
1. packet is not always available, e.g. when running pcre on http
buffers.
2. setting of vars should be done post match. Until now it was also
possible that it is done on a partial match.
Until now variable names, such as flowbit names, were local to a detect
engine. This made sense as they were only ever used in that context.
For the purpose of logging these names, this needs a different approach.
The loggers live outside of the detect engine. Also, in the case of
reloads and multi-tenancy, there are even multiple detect engines, so
it would be even more tricky to access them from the outside.
This patch brings a new approach. A any time, there is a single active
hash table mapping the variable names and their id's. For multiple
tenants the table is shared between tenants.
The table is set up in a 'staging' area, where locking makes sure that
multiple loading threads don't mess things up. Then when the preparing
of a detection engine is ready, but before the detect threads are made
aware of the new detect engine, the active varname hash is swapped with
the staging instance.
For this to work, all the mappings from the 'current' or active mapping
are added to the staging table.
After the threads have reloaded and the new detection engine is active,
the old table can be freed.
For multi tenancy things are similar. The staging area is used for
setting up until the new detection engines / tenants are applied to
the system.
This patch also changes the variable 'id'/'idx' field to uint32_t. Due
to data structure padding and alignment, this should have no practical
drawback while allowing for a lot more vars.
To simplify locking, move all locking out of the individual detect
code. Instead at the start of detection lock the flow, and at the
end of detection unlock it.
The lua code can be called without a lock still (from the output
code paths), so still pass around a lock hint to take care of this.
The post match list was called with an unlocked flow until now.
However, recent de_state handling updates changed this. The stateful
detection code can now call the post match functions while keeping
the flow locked. The normal detection code still calls it with an
unlocked flow.
This patch adds a hint to the DetectEngineThreadCtx called
'flow_locked' that is set to true if the caller has already locked
the flow.
The Match functions don't need a pointer to the SigMatch object, just the
context pointer contained inside, so pass the Context to the Match function
rather than the SigMatch object. This allows for further optimization.
Change SigMatch->ctx to have type SigMatchCtx* rather than void* for better
type checking. This requires adding type casts when using or assigning it.
The SigMatch contex should not be changed by the Match() funciton, so pass it
as a const SigMatchCtx*.
Creates an inline wrapper to check for flowvarlist == NULL before calling
DetectFlowvarProcessList() to remove the overhead of checking since the
list is usually empty.
Bug #802
Flowvars are set from pcre, and lock the flow when being set. However
when HTTP buffers were inspected, flow was already locked: deadlock.
This patch introduces a post-match list in the detection engine thread
ctx, where store candidates are kept. Then a post-match function is used
to finalize the storing if the rule matches.
Solves the deadlock and brings the handling of flowvars more in line
with flowbits and flowints.
When handling error case on SCMallog, SCCalloc or SCStrdup
we are in an unlikely case. This patch adds the unlikely()
expression to indicate this to gcc.
This patch has been obtained via coccinelle. The transformation
is the following:
@istested@
identifier x;
statement S1;
identifier func =~ "(SCMalloc|SCStrdup|SCCalloc)";
@@
x = func(...)
... when != x
- if (x == NULL) S1
+ if (unlikely(x == NULL)) S1
Shivani Bhardwaj
Philippe Antoine
Jeff Lucovsky
Victor Julien
Philippe Antoine
Victor Julien
Jeff Lucovsky
Ken Steele
Eric Leblond
Anoop Saldanha
Gurvinder Singh
William Metcalf