[Oisf-devel] [PATCH] Memory leak cleanup in detectors
Steve Grubb
sgrubb at redhat.com
Sun Jan 24 19:30:36 UTC 2010
Hello,
I ran the code through an analysis program and found several leaks that
should be cleaned up.
*In src/detect-engine-address-ipv4.c at line 472, the test for ag == NULL
will never be true since that is the loop entry test.
*In src/detect-engine-port.c at line 1133, the test for p == NULL will
never be true since that is the loop entry test.
*In src/detect-engine-mpm.c at line 263 is a return without freeing
fast_pattern
*In src/detect-ack.c at line 80 and 85, data catches the return from malloc.
One of them should be deleted.
*In src/detect-seq.c at line 81 and 86, data catches the return from malloc.
One of them should be deleted.
*In src/detect-content.c at line 749, many of the paths that lead to the error
exit still has temp pointing to allocated memory. To clean this up, temp
should be set to NULL if not immediately assigning and new value.
*In src/detect-uricontent.c at line 319, both cd and str needto be freed. At
lines 344, str needs to be freed. And at line 347 str and temp need to be
freed.
*In src/detect-flowbits.c at line 231 and 235, str was not being freed. cd was
not being freed at line 235.
*In src/detect-flowvar.c at line 127, str was not being freed. At line 194, cd
and str were not being freed.
*In src/detect-flowint.c at line 277, sfd was not being freed. At line 315, str
was not being freed.
*In src/detect-pktvar.c at line 121, str was not being freed. At line 188, str
and cd was not being freed.
*In src/detect-pcre.c at line 389, there is an extra free of "re" that should
be deleted.
*In src/detect-depth.c at line 42 & 48, str has not been freed.
*In src/detect-distance.c at line 49 and 55, str has not been freed
*In src/detect-offset.c at line 45, str has not been freed.
The patch below fixes these issues.
-Steve
diff -urp suricata-0.8.1.orig/src/detect-ack.c suricata-0.8.1/src/detect-ack.c
--- suricata-0.8.1.orig/src/detect-ack.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-ack.c 2010-01-24 11:38:39.000000000 -0500
@@ -77,7 +77,7 @@ static int DetectAckMatch(ThreadVars *t,
static int DetectAckSetup(DetectEngineCtx *de_ctx, Signature *s,
SigMatch *m, char *optstr)
{
- DetectAckData *data = malloc(sizeof(DetectAckData));
+ DetectAckData *data;
SigMatch *sm = NULL;
//printf("DetectAckSetup: \'%s\'\n", optstr);
diff -urp suricata-0.8.1.orig/src/detect-content.c suricata-0.8.1/src/detect-content.c
--- suricata-0.8.1.orig/src/detect-content.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-content.c 2010-01-24 12:28:25.000000000 -0500
@@ -591,7 +591,7 @@ DetectContentData *DetectContentParse (c
goto error;
if (strlen(temp) == 0) {
- if (temp) free(temp);
+ free(temp);
return NULL;
}
@@ -625,6 +625,7 @@ DetectContentData *DetectContentParse (c
}
free(temp);
+ temp = NULL;
if (str[0] == '!') {
if (cd->negated == 1) {
@@ -637,6 +638,7 @@ DetectContentData *DetectContentParse (c
goto error;
cd->negated = 1;
free(temp);
+ temp = NULL;
}
}
@@ -741,6 +743,7 @@ DetectContentData *DetectContentParse (c
error:
free(str);
+ free(temp);
if (cd != NULL) {
if (cd->content != NULL)
free(cd->content);
diff -urp suricata-0.8.1.orig/src/detect-depth.c suricata-0.8.1/src/detect-depth.c
--- suricata-0.8.1.orig/src/detect-depth.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-depth.c 2010-01-24 13:01:10.000000000 -0500
@@ -39,12 +39,14 @@ int DetectDepthSetup (DetectEngineCtx *d
SigMatch *pm = DetectContentFindPrevApplicableSM(m);
if (pm == NULL) {
SCLogError(SC_ERR_DEPTH_MISSING_CONTENT, "depth needs a preceeding content option");
+ if (dubbed) free(str);
return -1;
}
DetectContentData *cd = (DetectContentData *)pm->ctx;
if (cd == NULL) {
SCLogError(SC_INVALID_ARGUMENT, "invalid argument");
+ if (dubbed) free(str);
return -1;
}
diff -urp suricata-0.8.1.orig/src/detect-distance.c suricata-0.8.1/src/detect-distance.c
--- suricata-0.8.1.orig/src/detect-distance.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-distance.c 2010-01-24 13:04:19.000000000 -0500
@@ -46,12 +46,14 @@ int DetectDistanceSetup (DetectEngineCtx
pm = DetectContentFindPrevApplicableSM(m);
if (pm == NULL || DetectContentHasPrevSMPattern(pm) == NULL) {
SCLogError(SC_ERR_DISTANCE_MISSING_CONTENT, "distance needs two preceeding content options");
+ if (dubbed) free(str);
return -1;
}
DetectContentData *cd = (DetectContentData *)pm->ctx;
if (cd == NULL) {
printf("DetectDistanceSetup: Unknown previous keyword!\n");
+ if (dubbed) free(str);
return -1;
}
diff -urp suricata-0.8.1.orig/src/detect-engine-address-ipv4.c suricata-0.8.1/src/detect-engine-address-ipv4.c
--- suricata-0.8.1.orig/src/detect-engine-address-ipv4.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-engine-address-ipv4.c 2010-01-24 11:29:38.000000000 -0500
@@ -469,8 +469,6 @@ int DetectAddressIsCompleteIPSpaceIPv4(D
ag = ag->next;
for ( ; ag != NULL; ag = ag->next) {
- if (ag == NULL)
- return 0;
if (ag->ip[0] != next_ip)
return 0;
diff -urp suricata-0.8.1.orig/src/detect-engine-mpm.c suricata-0.8.1/src/detect-engine-mpm.c
--- suricata-0.8.1.orig/src/detect-engine-mpm.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-engine-mpm.c 2010-01-24 11:34:27.000000000 -0500
@@ -259,8 +259,10 @@ static int PatternMatchPreprarePopulateM
memset(fast_pattern, 0, sgh->sig_cnt * sizeof(uint32_t));
HashTable *ht = HashTableInit(4096, ContentHashFunc, ContentHashCompareFunc, ContentHashFree);
- if (ht == NULL)
+ if (ht == NULL) {
+ free(fast_pattern);
return -1;
+ }
/* add all the contents to a counting hash */
for (sig = 0; sig < sgh->sig_cnt; sig++) {
diff -urp suricata-0.8.1.orig/src/detect-engine-port.c suricata-0.8.1/src/detect-engine-port.c
--- suricata-0.8.1.orig/src/detect-engine-port.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-engine-port.c 2010-01-24 11:32:14.000000000 -0500
@@ -1130,9 +1130,6 @@ int DetectPortIsCompletePortSpace(Detect
p = p->next;
for ( ; p != NULL; p = p->next) {
- if (p == NULL)
- return 0;
-
if (p->port != next_port)
return 0;
diff -urp suricata-0.8.1.orig/src/detect-flowbits.c suricata-0.8.1/src/detect-flowbits.c
--- suricata-0.8.1.orig/src/detect-flowbits.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-flowbits.c 2010-01-24 12:41:10.000000000 -0500
@@ -228,12 +228,13 @@ int DetectFlowbitSetup (DetectEngineCtx
SigMatchAppend(s,m,sm);
- if (dubbed) free(str);
+ free(str);
return 0;
error:
- if (dubbed) free(str);
- if (sm) free(sm);
+ free(str);
+ free(cd);
+ free(sm);
return -1;
}
diff -urp suricata-0.8.1.orig/src/detect-flowint.c suricata-0.8.1/src/detect-flowint.c
--- suricata-0.8.1.orig/src/detect-flowint.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-flowint.c 2010-01-24 12:53:35.000000000 -0500
@@ -274,6 +274,7 @@ DetectFlowintData *DetectFlowintParse(De
varval =(char *) str_ptr;
if (res < 0 || strcmp(varval,"") == 0) {
SCLogDebug("DetectFlowintParse: pcre_get_substring failed");
+ free(sfd);
return NULL;
}
@@ -312,6 +313,7 @@ DetectFlowintData *DetectFlowintParse(De
return sfd;
error:
if (sfd != NULL) free(sfd);
+ free(str);
return NULL;
}
diff -urp suricata-0.8.1.orig/src/detect-flowvar.c suricata-0.8.1/src/detect-flowvar.c
--- suricata-0.8.1.orig/src/detect-flowvar.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-flowvar.c 2010-01-24 12:46:21.000000000 -0500
@@ -123,8 +123,10 @@ int DetectFlowvarSetup (DetectEngineCtx
}
len = strlen(str);
- if (len == 0)
+ if (len == 0) {
+ if (dubbed) free(str);
return -1;
+ }
cd = malloc(sizeof(DetectFlowvarData));
if (cd == NULL) {
@@ -190,8 +192,11 @@ int DetectFlowvarSetup (DetectEngineCtx
}
cd->content = malloc(len);
- if (cd->content == NULL)
+ if (cd->content == NULL) {
+ if (dubbed) free(str);
+ free(cd);
return -1;
+ }
cd->name = strdup(varname);
cd->idx = VariableNameGetIdx(de_ctx,varname,DETECT_FLOWVAR);
diff -urp suricata-0.8.1.orig/src/detect-offset.c suricata-0.8.1/src/detect-offset.c
--- suricata-0.8.1.orig/src/detect-offset.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-offset.c 2010-01-24 13:06:04.000000000 -0500
@@ -42,12 +42,14 @@ int DetectOffsetSetup (DetectEngineCtx *
SigMatch *pm = DetectContentFindPrevApplicableSM(m);
if (pm == NULL) {
SCLogError(SC_ERR_OFFSET_MISSING_CONTENT, "offset needs a preceeding content option");
+ if (dubbed) free(str);
return -1;
}
DetectContentData *cd = (DetectContentData *)pm->ctx;
if (cd == NULL) {
SCLogError(SC_INVALID_ARGUMENT, "invalid argument");
+ if (dubbed) free(str);
return -1;
}
diff -urp suricata-0.8.1.orig/src/detect-pcre.c suricata-0.8.1/src/detect-pcre.c
--- suricata-0.8.1.orig/src/detect-pcre.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-pcre.c 2010-01-24 12:59:19.000000000 -0500
@@ -386,7 +386,6 @@ error:
if (op_ptr != NULL) free(op_ptr);
if (pd != NULL && pd->re != NULL) pcre_free(pd->re);
if (pd != NULL && pd->sd != NULL) pcre_free(pd->sd);
- if (dubbed) free(re);
if (pd) free(pd);
return NULL;
}
diff -urp suricata-0.8.1.orig/src/detect-pktvar.c suricata-0.8.1/src/detect-pktvar.c
--- suricata-0.8.1.orig/src/detect-pktvar.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-pktvar.c 2010-01-24 12:57:50.000000000 -0500
@@ -117,8 +117,10 @@ int DetectPktvarSetup (DetectEngineCtx *
}
len = strlen(str);
- if (len == 0)
+ if (len == 0) {
+ if (dubbed) free(str);
return -1;
+ }
cd = malloc(sizeof(DetectPktvarData));
if (cd == NULL) {
@@ -184,8 +186,11 @@ int DetectPktvarSetup (DetectEngineCtx *
}
cd->content = malloc(len);
- if (cd->content == NULL)
+ if (cd->content == NULL) {
+ free(cd);
+ if (dubbed) free(str);
return -1;
+ }
cd->name = strdup(varname);
memcpy(cd->content, str, len);
diff -urp suricata-0.8.1.orig/src/detect-seq.c suricata-0.8.1/src/detect-seq.c
--- suricata-0.8.1.orig/src/detect-seq.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-seq.c 2010-01-24 11:39:31.000000000 -0500
@@ -78,7 +78,7 @@ static int DetectSeqMatch(ThreadVars *t,
static int DetectSeqSetup (DetectEngineCtx *de_ctx, Signature *s,
SigMatch *m, char *optstr)
{
- DetectSeqData *data = malloc(sizeof(DetectSeqData));
+ DetectSeqData *data;
SigMatch *sm = NULL;
//printf("DetectSeqSetup: \'%s\'\n", optstr);
diff -urp suricata-0.8.1.orig/src/detect-uricontent.c suricata-0.8.1/src/detect-uricontent.c
--- suricata-0.8.1.orig/src/detect-uricontent.c 2010-01-24 09:08:23.000000000 -0500
+++ suricata-0.8.1/src/detect-uricontent.c 2010-01-24 12:37:02.000000000 -0500
@@ -220,7 +220,7 @@ int DetectUricontentSetup (DetectEngineC
goto error;
if (strlen(temp) == 0) {
- if (temp) free(temp);
+ free(temp);
return -1;
}
@@ -252,6 +252,7 @@ int DetectUricontentSetup (DetectEngineC
}
free(temp);
+ temp = NULL;
len = strlen(str);
//printf("DetectUricontentSetup: \"%s\", len %" PRIu32 "\n", str, len);
@@ -315,8 +316,11 @@ int DetectUricontentSetup (DetectEngineC
SCLogDebug("len %" PRIu32 "", len);
cd->uricontent = malloc(len);
- if (cd->uricontent == NULL)
+ if (cd->uricontent == NULL) {
+ free(cd);
+ free(str);
return -1;
+ }
memcpy(cd->uricontent, str, len);
cd->uricontent_len = len;
@@ -340,11 +344,12 @@ int DetectUricontentSetup (DetectEngineC
cd->id = de_ctx->uricontent_max_id;
de_ctx->uricontent_max_id++;
- if (dubbed) free(str);
+ free(str);
return 0;
error:
- if (dubbed) free(str);
+ free(str);
+ free(temp);
if (cd) free(cd);
if (sm) free(sm);
return -1;
More information about the Oisf-devel
mailing list