[Oisf-devel] [PATCH] Memory leak cleanup in detectors
Victor Julien
victor at inliniac.net
Sun Jan 24 19:40:23 UTC 2010
Good catches. Applied, thanks.
Steve Grubb wrote:
> 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;
> _______________________________________________
> Oisf-devel mailing list
> Oisf-devel at openinfosecfoundation.org
> http://lists.openinfosecfoundation.org/mailman/listinfo/oisf-devel
--
---------------------------------------------
Victor Julien
http://www.inliniac.net/
PGP: http://www.inliniac.net/victorjulien.asc
---------------------------------------------
More information about the Oisf-devel
mailing list