Bug 169888 - Patch to fix memory leaks in logrotate
Patch to fix memory leaks in logrotate
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: logrotate (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Vrabec
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2005-10-04 14:48 EDT by Mateus César Gröess
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-10-18 10:20:21 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Fixes for memory leaks (2.13 KB, patch)
2005-10-04 14:48 EDT, Mateus César Gröess
no flags Details | Diff
fixes segfault introduced in previous patch + minor cleanup (2.47 KB, patch)
2005-10-13 22:18 EDT, trzareq
no flags Details | Diff
Patch to free memory allocated for tabooExts. (2.57 KB, patch)
2005-10-15 14:39 EDT, Mateus César Gröess
no flags Details | Diff
Same patch as before, with a little change to make readConfigPath static. (2.76 KB, patch)
2005-10-16 09:34 EDT, Mateus César Gröess
no flags Details | Diff
Maybe the final version of the patch to free tabooExts. (5.58 KB, patch)
2005-10-17 08:18 EDT, Mateus César Gröess
no flags Details | Diff

  None (edit)
Description Mateus César Gröess 2005-10-04 14:48:39 EDT
Patch attached that fixes some memory leaks (not all) of logrotate, reported by
Valgrind. Some things may look ugly, feel free to reject the patch. :-)

Known memory leaks not fixed by this patch are tabooExts in config.c:249, some
data of optCon in logrotate.c when poptGetNextOpt is called and the data of
argv[argNum] when called by glob in config.c:966.
Comment 1 Mateus César Gröess 2005-10-04 14:48:39 EDT
Created attachment 119620 [details]
Fixes for memory leaks
Comment 2 trzareq 2005-10-13 22:18:22 EDT
Created attachment 119955 [details]
fixes segfault introduced in previous patch + minor cleanup

Fixed segfault, but valgrind still reports lost blocks. Didn't have time to
debug and patch those. Sorry.
Comment 3 trzareq 2005-10-13 22:29:30 EDT
More info on the segfault:

Parts of the following block realloc'ed at config.c:

   *logsPtr = realloc(*logsPtr, sizeof(**logsPtr) * *numLogsPtr);

were passed free() in the patch at logrotate.c (free_loginfo):

    for (i = 0; i < *numLogsPtr; i++)
        free(logsPtr[i]);

Personally, I would consider including some C++ code to avoid similar problems
in the future in the logrotate package. Just an opinion though.
Comment 4 Mateus César Gröess 2005-10-15 14:39:53 EDT
Created attachment 120029 [details]
Patch to free memory allocated for tabooExts.

Thanks trzareq, you may see I am not a experienced programmer. I attached
another patch, this time to free memory allocated for tabooExts. There is still
one problem with tabooExts, because I have not found a proper solution to free
the memory allocated at end of the code below (of config.c), where is a comment
about it.
		if (!isolateValue(configFile, lineNum, "tabooext", &start,
				  &endtag)) {
		    oldchar = *endtag, *endtag = '\0';

		    if (*start == '+') {
			start++;
			while (isspace(*start) && *start) start++;
		    } else {
			free(tabooExts);
			tabooCount = 0;
			tabooExts = malloc(1);
		    }

		    while (*start) {
			chptr = start;
			while (!isspace(*chptr) && *chptr != ',' && *chptr)
			    chptr++;

			tabooExts = realloc(tabooExts, sizeof(*tabooExts) *
						(tabooCount + 1));
			/* this is a memory leak if the list gets reset */
			tabooExts[tabooCount] = malloc(chptr - start + 1);
			strncpy(tabooExts[tabooCount], start, chptr - start);
			tabooExts[tabooCount][chptr - start] = '\0';
			tabooCount++;

Apart from this problem, Valgrind only reports data leaked by poptGetNextOpt
(logrotate.c), then I think this maybe a problem with the popt library itself.
Comment 5 Mateus César Gröess 2005-10-16 09:34:52 EDT
Created attachment 120034 [details]
Same patch as before, with a little change to make readConfigPath static.
Comment 6 Mateus César Gröess 2005-10-17 08:18:50 EDT
Created attachment 120053 [details]
Maybe the final version of the patch to free tabooExts.

New version of the patch to fix leaks of tabooExts. I changed the way tabooExts
is allocated the first time, using malloc for each line of the array instead of
memcpy all pointers of defTabooExts. So I just free the array line-by-line,
when or not a new line is added by the code showed in the post above. To do
this I have renamed free_namelist to a more generic name (free_2d_array), for
use also with tabooExts, not only namelist. Valgrind now only reports leaks of
poptGetNextOpt (logrotate.c:1306), but there are calls to poptFreeContext for
every possible case, so I still think there is a problem with popt library.
Please, someone review this patch and, after applied it, close this bug.
Comment 7 Peter Vrabec 2005-10-18 10:20:21 EDT
I checked this patch. It seems to be right. Applied in logrotate-3.7.2-8

Note You need to log in before you can comment on or make changes to this bug.