Skip to content

Commit 4d5e27d

Browse files
committed
Speed up freeing of signatures
Speed up freeing of signatures by tracking all malloced blocks instead of having to find duplicates in our data structures on signature unload.
1 parent ece5375 commit 4d5e27d

2 files changed

Lines changed: 75 additions & 40 deletions

File tree

libclamav/matcher-ac.c

Lines changed: 70 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,57 @@ static void link_lists(struct cli_matcher *root)
305305
}
306306
}
307307

308+
/**
309+
* @brief Inserts newly malloced trans node in the array of nodes to be freed on
310+
* cleanup. There is no verification that the added node is not already in the
311+
* list, so that is up to the caller.
312+
*
313+
* @param root The matcher root.
314+
* @param trans The trans node to be tracked.
315+
* @return bool
316+
*/
317+
static bool store_trans_node(struct cli_matcher *root, void *trans)
318+
{
319+
bool bRet = false;
320+
321+
if (root->trans_cnt + 1 > root->trans_capacity) {
322+
uint64_t newCapacity = root->trans_capacity + 1024;
323+
void *ret = MPOOL_REALLOC(root->mempool, root->trans_array, newCapacity * sizeof(void *));
324+
if (NULL == ret) {
325+
cli_errmsg("cli_ac_addpatt: Can't allocate memory for cleanup storage of trans\n");
326+
goto done;
327+
}
328+
root->trans_capacity = newCapacity;
329+
root->trans_array = ret;
330+
}
331+
332+
root->trans_array[root->trans_cnt++] = trans;
333+
334+
bRet = true;
335+
done:
336+
return bRet;
337+
}
338+
339+
/**
340+
* @brief Frees all trans nodes for cleanup.
341+
* cleanup.
342+
*
343+
* @param root The matcher root.
344+
*/
345+
static void free_trans_nodes(struct cli_matcher *root)
346+
{
347+
uint32_t i = 0;
348+
349+
for (i = 0; i < root->trans_cnt; i++) {
350+
MPOOL_FREE(root->mempool, root->trans_array[i]);
351+
}
352+
353+
MPOOL_FREE(root->mempool, root->trans_array);
354+
root->trans_array = NULL;
355+
root->trans_cnt = 0;
356+
root->trans_capacity = 0;
357+
}
358+
308359
static inline struct cli_ac_node *add_new_node(struct cli_matcher *root, uint16_t i, uint16_t len)
309360
{
310361
struct cli_ac_node *new;
@@ -323,6 +374,12 @@ static inline struct cli_ac_node *add_new_node(struct cli_matcher *root, uint16_
323374
MPOOL_FREE(root->mempool, new);
324375
return NULL;
325376
}
377+
378+
if (!store_trans_node(root, new->trans)) {
379+
/* Error printed in store_trans_node */
380+
MPOOL_FREE(root->mempool, new);
381+
return NULL;
382+
}
326383
}
327384

328385
root->ac_nodes++;
@@ -359,6 +416,10 @@ static int cli_ac_addpatt_recursive(struct cli_matcher *root, struct cli_ac_patt
359416
cli_errmsg("cli_ac_addpatt: Can't allocate memory for pt->trans\n");
360417
return CL_EMEM;
361418
}
419+
if (!store_trans_node(root, pt->trans)) {
420+
/* Error printed in store_trans_node */
421+
return CL_EMEM;
422+
}
362423
}
363424

364425
/* if pattern is nocase, we need to enumerate all the combinations if applicable
@@ -654,32 +715,6 @@ static void ac_free_special(struct cli_ac_patt *p)
654715
MPOOL_FREE(mempool, p->special_table);
655716
}
656717

657-
/*
658-
* This is a test to see if we have already seen this pointer. If we have, we
659-
* have already freed it, so don't do it again (double-free)
660-
*/
661-
static int need_to_free_trans(struct cli_matcher *root, const size_t idx)
662-
{
663-
size_t j;
664-
size_t min = idx;
665-
if (root->ac_nodes < idx) {
666-
/*Should never happen, but check just to be safe.*/
667-
min = root->ac_nodes;
668-
}
669-
670-
for (j = 0; j < min; j++) {
671-
if (NULL == root->ac_nodetable[j]) {
672-
continue;
673-
}
674-
675-
if (root->ac_nodetable[idx]->trans == root->ac_nodetable[j]->trans) {
676-
return 0;
677-
}
678-
}
679-
680-
return 1;
681-
}
682-
683718
void cli_ac_free(struct cli_matcher *root)
684719
{
685720
uint32_t i = 0;
@@ -708,17 +743,6 @@ void cli_ac_free(struct cli_matcher *root)
708743
MPOOL_FREE(root->mempool, root->ac_reloff);
709744
}
710745

711-
/* Freeing trans nodes must be done before freeing table nodes! */
712-
for (i = 0; i < root->ac_nodes; i++) {
713-
if (!IS_LEAF(root->ac_nodetable[i]) &&
714-
root->ac_root->trans != root->ac_nodetable[i]->trans) {
715-
716-
if (need_to_free_trans(root, i)) {
717-
MPOOL_FREE(root->mempool, root->ac_nodetable[i]->trans);
718-
}
719-
}
720-
}
721-
722746
for (i = 0; i < root->ac_lists; i++) {
723747
MPOOL_FREE(root->mempool, root->ac_listtable[i]);
724748
}
@@ -743,6 +767,8 @@ void cli_ac_free(struct cli_matcher *root)
743767
if (root->filter) {
744768
MPOOL_FREE(root->mempool, root->filter);
745769
}
770+
771+
free_trans_nodes(root);
746772
}
747773

748774
/*
@@ -2301,9 +2327,13 @@ inline static int ac_uicmp(uint16_t *a, size_t alen, uint16_t *b, size_t blen, i
23012327
/* add new generic alternate node to special */
23022328
inline static int ac_addspecial_add_alt_node(const char *subexpr, uint8_t sigopts, struct cli_ac_special *special, struct cli_matcher *root)
23032329
{
2304-
struct cli_alt_node *newnode, **prev, *ins;
2305-
uint16_t *s;
2306-
int i, cmp, wild;
2330+
struct cli_alt_node *newnode = NULL;
2331+
struct cli_alt_node **prev = NULL;
2332+
struct cli_alt_node *ins = NULL;
2333+
uint16_t *s = NULL;
2334+
int i = 0;
2335+
int cmp = 0;
2336+
int wild = 0;
23072337

23082338
#ifndef USE_MPOOL
23092339
UNUSEDPARAM(root);

libclamav/matcher.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,11 @@ struct cli_matcher {
174174
/* Bytecode Tracker */
175175
uint32_t linked_bcs;
176176

177+
/*Store pointers to malloced trans values so that they can be more easily freed*/
178+
struct cli_ac_node **trans_array;
179+
size_t trans_cnt;
180+
size_t trans_capacity;
181+
177182
#ifdef USE_MPOOL
178183
mpool_t *mempool;
179184
#else

0 commit comments

Comments
 (0)