-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gen/refactor: transform and misc related changes for improved transform support #12502
base: master
Are you sure you want to change the base?
Changes from 5 commits
5cf7ba4
fbadecd
1de19ed
cfb7d18
a03b777
1161082
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -960,14 +960,15 @@ static char DetectBufferTypeCompareIdFunc(void *data1, uint16_t len1, void *data | |
return map1->id == map2->id; | ||
} | ||
|
||
static void DetectBufferTypeFreeFunc(void *data) | ||
static void DetectBufferTypeFreeFuncWithCtx(void *ctx, void *data) | ||
{ | ||
DetectBufferType *map = (DetectBufferType *)data; | ||
|
||
if (map == NULL) { | ||
if (data == NULL) { | ||
return; | ||
} | ||
|
||
DetectBufferType *map = (DetectBufferType *)data; | ||
DetectEngineCtx *de_ctx = (DetectEngineCtx *)ctx; | ||
|
||
/* Release transformation option memory, if any */ | ||
for (int i = 0; i < map->transforms.cnt; i++) { | ||
if (map->transforms.transforms[i].options == NULL) | ||
|
@@ -977,12 +978,18 @@ static void DetectBufferTypeFreeFunc(void *data) | |
sigmatch_table[map->transforms.transforms[i].transform].name); | ||
continue; | ||
} | ||
sigmatch_table[map->transforms.transforms[i].transform].Free(NULL, map->transforms.transforms[i].options); | ||
sigmatch_table[map->transforms.transforms[i].transform].Free( | ||
de_ctx, map->transforms.transforms[i].options); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks ok but feels painful Why will we need this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Luaxform will need the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this thread ctx ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another option would be to have |
||
} | ||
|
||
SCFree(map); | ||
} | ||
|
||
static void DetectBufferTypeFreeFunc(void *data) | ||
{ | ||
DetectBufferTypeFreeFuncWithCtx(NULL, data); | ||
} | ||
|
||
static int DetectBufferTypeInit(void) | ||
{ | ||
BUG_ON(g_buffer_type_hash); | ||
|
@@ -1580,8 +1587,8 @@ void InspectionBufferSetupMultiEmpty(InspectionBuffer *buffer) | |
} | ||
|
||
/** \brief setup the buffer with our initial data */ | ||
void InspectionBufferSetupMulti(InspectionBuffer *buffer, const DetectEngineTransforms *transforms, | ||
const uint8_t *data, const uint32_t data_len) | ||
void InspectionBufferSetupMulti(DetectEngineThreadCtx *det_ctx, InspectionBuffer *buffer, | ||
const DetectEngineTransforms *transforms, const uint8_t *data, const uint32_t data_len) | ||
{ | ||
#ifdef DEBUG_VALIDATION | ||
DEBUG_VALIDATE_BUG_ON(!buffer->multi); | ||
|
@@ -1591,7 +1598,7 @@ void InspectionBufferSetupMulti(InspectionBuffer *buffer, const DetectEngineTran | |
buffer->len = 0; | ||
buffer->initialized = true; | ||
|
||
InspectionBufferApplyTransforms(buffer, transforms); | ||
InspectionBufferApplyTransforms(det_ctx, buffer, transforms); | ||
} | ||
|
||
/** \brief setup the buffer with our initial data */ | ||
|
@@ -1711,7 +1718,7 @@ bool DetectEngineBufferTypeValidateTransform(DetectEngineCtx *de_ctx, int sm_lis | |
return true; | ||
} | ||
|
||
void InspectionBufferApplyTransforms(InspectionBuffer *buffer, | ||
void InspectionBufferApplyTransforms(DetectEngineThreadCtx *det_ctx, InspectionBuffer *buffer, | ||
const DetectEngineTransforms *transforms) | ||
{ | ||
if (transforms) { | ||
|
@@ -1720,7 +1727,7 @@ void InspectionBufferApplyTransforms(InspectionBuffer *buffer, | |
if (id == 0) | ||
break; | ||
BUG_ON(sigmatch_table[id].Transform == NULL); | ||
sigmatch_table[id].Transform(buffer, transforms->transforms[i].options); | ||
sigmatch_table[id].Transform(det_ctx, buffer, transforms->transforms[i].options); | ||
SCLogDebug("applied transform %s", sigmatch_table[id].name); | ||
} | ||
} | ||
|
@@ -1731,8 +1738,8 @@ static void DetectBufferTypeSetupDetectEngine(DetectEngineCtx *de_ctx) | |
const int size = g_buffer_type_id; | ||
BUG_ON(!(size > 0)); | ||
|
||
de_ctx->buffer_type_hash_name = HashListTableInit(256, DetectBufferTypeHashNameFunc, | ||
DetectBufferTypeCompareNameFunc, DetectBufferTypeFreeFunc); | ||
de_ctx->buffer_type_hash_name = HashListTableInitWithCtx(256, DetectBufferTypeHashNameFunc, | ||
DetectBufferTypeCompareNameFunc, DetectBufferTypeFreeFuncWithCtx); | ||
BUG_ON(de_ctx->buffer_type_hash_name == NULL); | ||
de_ctx->buffer_type_hash_id = | ||
HashListTableInit(256, DetectBufferTypeHashIdFunc, DetectBufferTypeCompareIdFunc, | ||
|
@@ -1773,7 +1780,7 @@ static void DetectBufferTypeFreeDetectEngine(DetectEngineCtx *de_ctx) | |
{ | ||
if (de_ctx) { | ||
if (de_ctx->buffer_type_hash_name) | ||
HashListTableFree(de_ctx->buffer_type_hash_name); | ||
HashListTableFreeWithCtx(de_ctx, de_ctx->buffer_type_hash_name); | ||
if (de_ctx->buffer_type_hash_id) | ||
HashListTableFree(de_ctx->buffer_type_hash_id); | ||
|
||
|
@@ -2599,6 +2606,9 @@ DetectEngineCtx *DetectEngineCtxInitWithPrefix(const char *prefix, uint32_t tena | |
static void DetectEngineCtxFreeThreadKeywordData(DetectEngineCtx *de_ctx) | ||
{ | ||
HashListTableFree(de_ctx->keyword_hash); | ||
#if UNITTESTS | ||
de_ctx->keyword_hash = NULL; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Double free Happens only in unit tests, right ? Just double checking There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And why was CI green if there was this double free ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is a unit test only situation. The double free would occur in DetectUnregsiterThreadCtxFuncs iff keyword hash wasn't nullified. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And why was CI green if there was this double free ? |
||
#endif | ||
} | ||
|
||
static void DetectEngineCtxFreeFailedSigs(DetectEngineCtx *de_ctx) | ||
|
@@ -2671,7 +2681,6 @@ void DetectEngineCtxFree(DetectEngineCtx *de_ctx) | |
|
||
MpmFactoryDeRegisterAllMpmCtxProfiles(de_ctx); | ||
|
||
DetectEngineCtxFreeThreadKeywordData(de_ctx); | ||
SRepDestroy(de_ctx); | ||
DetectEngineCtxFreeFailedSigs(de_ctx); | ||
|
||
|
@@ -2694,6 +2703,7 @@ void DetectEngineCtxFree(DetectEngineCtx *de_ctx) | |
DetectPortCleanupList(de_ctx, de_ctx->udp_priorityports); | ||
|
||
DetectBufferTypeFreeDetectEngine(de_ctx); | ||
DetectEngineCtxFreeThreadKeywordData(de_ctx); | ||
SCClassConfDeinit(de_ctx); | ||
SCReferenceConfDeinit(de_ctx); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not deserve its own commit :-p