Skip to content
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.

Sign up for GitHub

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

Jump to bottom

detect/multi-tenant: Eliminate crashes processing tenant config #9060

Closed
wants to merge 3 commits into from
Closed

detect/multi-tenant: Eliminate crashes processing tenant config #9060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ccdaa6
detect/multi-tenant: Make tenant_id 32 bits everywhere
jlucovsky Jun 21, 2023
0721a5e
detect/config: Serialize config file processing
jlucovsky Jun 21, 2023
b8c5728
detect/alloc: Use SCCalloc instead of SCMalloc
jlucovsky Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 9 additions & 17 deletions src/detect-engine.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,11 @@ void DetectAppLayerInspectEngineRegister2(const char *name,
direction = 1;
}

DetectEngineAppInspectionEngine *new_engine = SCMalloc(sizeof(DetectEngineAppInspectionEngine));
DetectEngineAppInspectionEngine *new_engine =
SCCalloc(1, sizeof(DetectEngineAppInspectionEngine));
if (unlikely(new_engine == NULL)) {
exit(EXIT_FAILURE);
}
memset(new_engine, 0, sizeof(*new_engine));
new_engine->alproto = alproto;
new_engine->dir = direction;
new_engine->sm_list = (uint16_t)sm_list;
Expand Down Expand Up @@ -2459,11 +2459,10 @@ static int DetectEngineReloadThreads(DetectEngineCtx *new_de_ctx)

static DetectEngineCtx *DetectEngineCtxInitReal(enum DetectEngineType type, const char *prefix)
{
DetectEngineCtx *de_ctx = SCMalloc(sizeof(DetectEngineCtx));
DetectEngineCtx *de_ctx = SCCalloc(1, sizeof(DetectEngineCtx));
if (unlikely(de_ctx == NULL))
goto error;

memset(de_ctx,0,sizeof(DetectEngineCtx));
memset(&de_ctx->sig_stat, 0, sizeof(SigFileLoaderStat));
TAILQ_INIT(&de_ctx->sig_stat.failed_sigs);
de_ctx->sigerror = NULL;
Expand Down Expand Up @@ -3028,14 +3027,12 @@ static int DetectEngineThreadCtxInitKeywords(DetectEngineCtx *de_ctx, DetectEngi
{
if (de_ctx->keyword_id > 0) {
// coverity[suspicious_sizeof : FALSE]
det_ctx->keyword_ctxs_array = SCMalloc(de_ctx->keyword_id * sizeof(void *));
det_ctx->keyword_ctxs_array = SCCalloc(1, de_ctx->keyword_id * sizeof(void *));
if (det_ctx->keyword_ctxs_array == NULL) {
SCLogError("setting up thread local detect ctx");
return TM_ECODE_FAILED;
}

memset(det_ctx->keyword_ctxs_array, 0x00, de_ctx->keyword_id * sizeof(void *));

det_ctx->keyword_ctxs_size = de_ctx->keyword_id;

HashListTableBucket *hb = HashListTableGetListHead(de_ctx->keyword_hash);
Expand Down Expand Up @@ -3077,7 +3074,7 @@ static TmEcode DetectEngineThreadCtxInitForMT(ThreadVars *tv, DetectEngineThread
DetectEngineTenantMapping *map_array = NULL;
uint32_t map_array_size = 0;
uint32_t map_cnt = 0;
int max_tenant_id = 0;
uint32_t max_tenant_id = 0;
DetectEngineCtx *list = master->list;
HashTable *mt_det_ctxs_hash = NULL;

Expand Down Expand Up @@ -3212,13 +3209,10 @@ static TmEcode ThreadCtxDoInit (DetectEngineCtx *de_ctx, DetectEngineThreadCtx *
/* DeState */
if (de_ctx->sig_array_len > 0) {
det_ctx->match_array_len = de_ctx->sig_array_len;
det_ctx->match_array = SCMalloc(det_ctx->match_array_len * sizeof(Signature *));
det_ctx->match_array = SCCalloc(1, det_ctx->match_array_len * sizeof(Signature *));
if (det_ctx->match_array == NULL) {
return TM_ECODE_FAILED;
}
memset(det_ctx->match_array, 0,
det_ctx->match_array_len * sizeof(Signature *));

RuleMatchCandidateTxArrayInit(det_ctx, de_ctx->sig_array_len);
}

Expand Down Expand Up @@ -3299,10 +3293,9 @@ static TmEcode ThreadCtxDoInit (DetectEngineCtx *de_ctx, DetectEngineThreadCtx *
*/
TmEcode DetectEngineThreadCtxInit(ThreadVars *tv, void *initdata, void **data)
{
DetectEngineThreadCtx *det_ctx = SCMalloc(sizeof(DetectEngineThreadCtx));
DetectEngineThreadCtx *det_ctx = SCCalloc(1, sizeof(DetectEngineThreadCtx));
if (unlikely(det_ctx == NULL))
return TM_ECODE_FAILED;
memset(det_ctx, 0, sizeof(DetectEngineThreadCtx));

det_ctx->tv = tv;
det_ctx->de_ctx = DetectEngineGetCurrent();
Expand Down Expand Up @@ -3365,10 +3358,9 @@ TmEcode DetectEngineThreadCtxInit(ThreadVars *tv, void *initdata, void **data)
DetectEngineThreadCtx *DetectEngineThreadCtxInitForReload(
ThreadVars *tv, DetectEngineCtx *new_de_ctx, int mt)
{
DetectEngineThreadCtx *det_ctx = SCMalloc(sizeof(DetectEngineThreadCtx));
DetectEngineThreadCtx *det_ctx = SCCalloc(1, sizeof(DetectEngineThreadCtx));
if (unlikely(det_ctx == NULL))
return NULL;
memset(det_ctx, 0, sizeof(DetectEngineThreadCtx));

det_ctx->tenant_id = new_de_ctx->tenant_id;
det_ctx->tv = tv;
Expand Down Expand Up @@ -4426,7 +4418,7 @@ static uint32_t DetectEngineTenantGetIdFromPcap(const void *ctx, const Packet *p
return p->pcap_v.tenant_id;
}

DetectEngineCtx *DetectEngineGetByTenantId(int tenant_id)
DetectEngineCtx *DetectEngineGetByTenantId(uint32_t tenant_id)
{
DetectEngineMasterCtx *master = &g_master_de_ctx;
SCMutexLock(&master->lock);
Expand Down
2 changes: 1 addition & 1 deletion src/detect-engine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ uint32_t DetectEngineGetVersion(void);
void DetectEngineBumpVersion(void);
int DetectEngineAddToMaster(DetectEngineCtx *de_ctx);
DetectEngineCtx *DetectEngineGetCurrent(void);
DetectEngineCtx *DetectEngineGetByTenantId(int tenant_id);
DetectEngineCtx *DetectEngineGetByTenantId(uint32_t tenant_id);
void DetectEnginePruneFreeList(void);
int DetectEngineMoveToFreeList(DetectEngineCtx *de_ctx);
DetectEngineCtx *DetectEngineReference(DetectEngineCtx *);
Expand Down
2 changes: 1 addition & 1 deletion src/detect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,7 @@ enum DetectEngineType
typedef struct DetectEngineCtx_ {
int failure_fatal;

int tenant_id;
uint32_t tenant_id;

Signature *sig_list;
uint32_t sig_cnt;
Expand Down
26 changes: 9 additions & 17 deletions src/runmode-unix-socket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ int unix_socket_mode_is_running = 0;
typedef struct PcapFiles_ {
char *filename;
char *output_dir;
int tenant_id;
uint32_t tenant_id;
time_t delay;
time_t poll_interval;
bool continuous;
Expand Down Expand Up @@ -265,16 +265,8 @@ static void PcapFilesFree(PcapFiles *cfile)
*
* \retval 0 in case of error, 1 in case of success
*/
static TmEcode UnixListAddFile(
PcapCommand *this,
const char *filename,
const char *output_dir,
int tenant_id,
bool continuous,
bool should_delete,
time_t delay,
time_t poll_interval
)
static TmEcode UnixListAddFile(PcapCommand *this, const char *filename, const char *output_dir,
uint32_t tenant_id, bool continuous, bool should_delete, time_t delay, time_t poll_interval)
{
PcapFiles *cfile = NULL;
if (filename == NULL || this == NULL)
Expand Down Expand Up @@ -327,7 +319,7 @@ static TmEcode UnixSocketAddPcapFileImpl(json_t *cmd, json_t* answer, void *data
PcapCommand *this = (PcapCommand *) data;
const char *filename;
const char *output_dir;
int tenant_id = 0;
uint32_t tenant_id = 0;
bool should_delete = false;
time_t delay = 30;
time_t poll_interval = 5;
Expand Down Expand Up @@ -876,7 +868,7 @@ TmEcode UnixSocketRegisterTenantHandler(json_t *cmd, json_t* answer, void *data)
json_object_set_new(answer, "message", json_string("id is not an integer"));
return TM_ECODE_FAILED;
}
int tenant_id = json_integer_value(jarg);
uint32_t tenant_id = json_integer_value(jarg);

/* 2 get tenant handler type */
jarg = json_object_get(cmd, "htype");
Expand Down Expand Up @@ -957,7 +949,7 @@ TmEcode UnixSocketUnregisterTenantHandler(json_t *cmd, json_t* answer, void *dat
json_object_set_new(answer, "message", json_string("id is not an integer"));
return TM_ECODE_FAILED;
}
int tenant_id = json_integer_value(jarg);
uint32_t tenant_id = json_integer_value(jarg);

/* 2 get tenant handler type */
jarg = json_object_get(cmd, "htype");
Expand Down Expand Up @@ -1042,7 +1034,7 @@ TmEcode UnixSocketRegisterTenant(json_t *cmd, json_t* answer, void *data)
json_object_set_new(answer, "message", json_string("id is not an integer"));
return TM_ECODE_FAILED;
}
int tenant_id = json_integer_value(jarg);
uint32_t tenant_id = json_integer_value(jarg);

/* 2 get tenant yaml */
jarg = json_object_get(cmd, "filename");
Expand Down Expand Up @@ -1118,7 +1110,7 @@ TmEcode UnixSocketReloadTenant(json_t *cmd, json_t* answer, void *data)
json_object_set_new(answer, "message", json_string("id is not an integer"));
return TM_ECODE_FAILED;
}
int tenant_id = json_integer_value(jarg);
uint32_t tenant_id = json_integer_value(jarg);

/* 2 get tenant yaml */
jarg = json_object_get(cmd, "filename");
Expand Down Expand Up @@ -1188,7 +1180,7 @@ TmEcode UnixSocketUnregisterTenant(json_t *cmd, json_t* answer, void *data)
json_object_set_new(answer, "message", json_string("id is not an integer"));
return TM_ECODE_FAILED;
}
int tenant_id = json_integer_value(jarg);
uint32_t tenant_id = json_integer_value(jarg);

SCLogInfo("remove-tenant: removing tenant %d", tenant_id);

Expand Down
9 changes: 8 additions & 1 deletion src/util-classification-config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
static pcre2_code *regex = NULL;
static pcre2_match_data *regex_match = NULL;

static SCMutex class_mutex = SCMUTEX_INITIALIZER;

uint32_t SCClassConfClasstypeHashFunc(HashTable *ht, void *data, uint16_t datalen);
char SCClassConfClasstypeHashCompareFunc(void *data1, uint16_t datalen1,
void *data2, uint16_t datalen2);
Expand Down Expand Up @@ -91,6 +93,8 @@ void SCClassConfDeinit(void)
pcre2_match_data_free(regex_match);
regex_match = NULL;
}

SCMutexDestroy(&class_mutex);
}


Expand Down Expand Up @@ -248,6 +252,7 @@ int SCClassConfAddClasstype(DetectEngineCtx *de_ctx, char *rawstr, uint16_t inde

int ret = 0;

SCMutexLock(&class_mutex);
ret = pcre2_match(regex, (PCRE2_SPTR8)rawstr, strlen(rawstr), 0, 0, regex_match, NULL);
if (ret < 0) {
SCLogError("Invalid Classtype in "
Expand Down Expand Up @@ -301,10 +306,12 @@ int SCClassConfAddClasstype(DetectEngineCtx *de_ctx, char *rawstr, uint16_t inde
SCFree(ct_new);
}

SCMutexUnlock(&class_mutex);
return 0;

error:
return -1;
SCMutexUnlock(&class_mutex);
return -1;
}

/**
Expand Down
11 changes: 10 additions & 1 deletion src/util-reference-config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
static pcre2_code *regex = NULL;
static pcre2_match_data *regex_match = NULL;

static SCMutex ref_mutex = SCMUTEX_INITIALIZER;

/* the hash functions */
uint32_t SCRConfReferenceHashFunc(HashTable *ht, void *data, uint16_t datalen);
char SCRConfReferenceHashCompareFunc(void *data1, uint16_t datalen1,
Expand All @@ -70,6 +72,8 @@ void SCReferenceConfInit(void)
}
regex_match = pcre2_match_data_create_from_pattern(regex, NULL);

SCMutexInit(&ref_mutex, NULL);

return;
}

Expand All @@ -83,6 +87,8 @@ void SCReferenceConfDeinit(void)
pcre2_match_data_free(regex_match);
regex_match = NULL;
}

SCMutexDestroy(&ref_mutex);
}


Expand Down Expand Up @@ -235,6 +241,7 @@ int SCRConfAddReference(DetectEngineCtx *de_ctx, const char *line)

int ret = 0;

SCMutexLock(&ref_mutex);
ret = pcre2_match(regex, (PCRE2_SPTR8)line, strlen(line), 0, 0, regex_match, NULL);
if (ret < 0) {
SCLogError("Invalid Reference Config in "
Expand Down Expand Up @@ -275,10 +282,12 @@ int SCRConfAddReference(DetectEngineCtx *de_ctx, const char *line)
SCRConfDeAllocSCRConfReference(ref_new);
}

SCMutexUnlock(&ref_mutex);
return 0;

error:
return -1;
SCMutexUnlock(&ref_mutex);
return -1;
}

/**
Expand Down
7 changes: 7 additions & 0 deletions src/util-threshold-config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ static pcre2_match_data *regex_rate_match = NULL;
static pcre2_code *regex_suppress = NULL;
static pcre2_match_data *regex_suppress_match = NULL;

static SCMutex thresh_mutex = SCMUTEX_INITIALIZER;

static void SCThresholdConfDeInitContext(DetectEngineCtx *de_ctx, FILE *fd);

void SCThresholdConfGlobalInit(void)
Expand Down Expand Up @@ -189,6 +191,8 @@ void SCThresholdConfGlobalFree(void)
pcre2_match_data_free(regex_suppress_match);
regex_suppress_match = NULL;
}

SCMutexDestroy(&thresh_mutex);
}

/**
Expand Down Expand Up @@ -661,6 +665,7 @@ static int ParseThresholdRule(const DetectEngineCtx *de_ctx, char *rawstr, uint3
if (de_ctx == NULL)
return -1;

SCMutexLock(&thresh_mutex);
ret = pcre2_match(
regex_base, (PCRE2_SPTR8)rawstr, strlen(rawstr), 0, 0, regex_base_match, NULL);
if (ret < 4) {
Expand Down Expand Up @@ -954,10 +959,12 @@ static int ParseThresholdRule(const DetectEngineCtx *de_ctx, char *rawstr, uint3
if (th_ip != NULL) {
*ret_th_ip = (char *)th_ip;
}
SCMutexUnlock(&thresh_mutex);
pcre2_substring_free((PCRE2_UCHAR8 *)rule_extend);
return 0;

error:
SCMutexUnlock(&thresh_mutex);
if (rule_extend != NULL) {
pcre2_substring_free((PCRE2_UCHAR8 *)rule_extend);
}
Expand Down