[PATCH] Fix potential memoryleak in guc.c

Started by Zhang, Jieover 6 years ago2 messages
#1Zhang, Jie
zhangjie2@cn.fujitsu.com
1 attachment(s)

Hi all

In src\backend\utils\misc\guc.c, I found a potential memory leak.

make_absolute_path() return a malloc'd copy, we should free memory before the function return false.
----------------------------------------------------------------------------
SelectConfigFiles(const char *userDoption, const char *progname)
{
......
/* configdir is -D option, or $PGDATA if no -D */
if (userDoption)
configdir = make_absolute_path(userDoption); ★
else
configdir = make_absolute_path(getenv("PGDATA")); ★

if (configdir && stat(configdir, &stat_buf) != 0)
{
write_stderr("%s: could not access directory \"%s\": %s\n",
progname,
configdir,
strerror(errno));
if (errno == ENOENT)
write_stderr("Run initdb or pg_basebackup to initialize a PostgreSQL data directory.\n");
★// Need to free memory of configdir
return false;
}
......
---------------------------------------------------------------------------

Refer to the following files for the implementation of make_absolute_path().

file: src\port\path.c
/*
* make_absolute_path
*
* If the given pathname isn't already absolute, make it so, interpreting
* it relative to the current working directory.
*
* Also canonicalizes the path. The result is always a malloc'd copy.

Attachments:

guc.patchapplication/octet-stream; name=guc.patchDownload
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 1208eb9..1fec8c9 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -5370,6 +5370,7 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 					 strerror(errno));
 		if (errno == ENOENT)
 			write_stderr("Run initdb or pg_basebackup to initialize a PostgreSQL data directory.\n");
+		free(configdir);
 		return false;
 	}
 
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zhang, Jie (#1)
Re: [PATCH] Fix potential memoryleak in guc.c

"Zhang, Jie" <zhangjie2@cn.fujitsu.com> writes:

In src\backend\utils\misc\guc.c, I found a potential memory leak.
make_absolute_path() return a malloc'd copy, we should free memory before the function return false.

If SelectConfigFiles were executed more than once per postmaster
launch, this might be worth adding code for ... but as-is, I'm
dubious. There are a few tens of KB of other one-time leaks
that we don't worry about removing.

Even more to the point, the particular code path you're complaining
about is a failure exit that will lead to immediate process
termination, so there really is no point in adding code there.

regards, tom lane