From b1d55dca917c8595eb89bae5a93c5f81c8b10f81 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 19 Nov 2009 02:45:33 +0000
Subject: [PATCH] Fix memory leak in syslogger: logfile_rotate() would leak a
 copy of the output filename if CSV logging was enabled and only one of the
 two possible output files got rotated during a particular call (which would,
 in fact, typically be the case during a size-based rotation).  This would
 amount to about MAXPGPATH (1KB) per rotation, and it's been there since the
 CSV code was put in, so it's surprising that nobody noticed it before. Per
 bug #5196 from Thomas Poindessous.

---
 src/backend/postmaster/syslogger.c | 50 ++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 17 deletions(-)

diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index c495c2487d3..0b49a81f954 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -18,7 +18,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/postmaster/syslogger.c,v 1.52 2009/11/05 20:13:06 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/postmaster/syslogger.c,v 1.53 2009/11/19 02:45:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -140,7 +140,7 @@ static void open_csvlogfile(void);
 static unsigned int __stdcall pipeThread(void *arg);
 #endif
 static void logfile_rotate(bool time_based_rotation, int size_rotation_for);
-static char *logfile_getname(pg_time_t timestamp, char *suffix);
+static char *logfile_getname(pg_time_t timestamp, const char *suffix);
 static void set_next_rotation_time(void);
 static void sigHupHandler(SIGNAL_ARGS);
 static void sigUsr1Handler(SIGNAL_ARGS);
@@ -1016,6 +1016,7 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 {
 	char	   *filename;
 	char	   *csvfilename = NULL;
+	pg_time_t	fntime;
 	FILE	   *fh;
 
 	rotation_requested = false;
@@ -1026,17 +1027,12 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 	 * file name when we don't do the rotation immediately.
 	 */
 	if (time_based_rotation)
-	{
-		filename = logfile_getname(next_rotation_time, NULL);
-		if (csvlogFile != NULL)
-			csvfilename = logfile_getname(next_rotation_time, ".csv");
-	}
+		fntime = next_rotation_time;
 	else
-	{
-		filename = logfile_getname(time(NULL), NULL);
-		if (csvlogFile != NULL)
-			csvfilename = logfile_getname(time(NULL), ".csv");
-	}
+		fntime = time(NULL);
+	filename = logfile_getname(fntime, NULL);
+	if (csvlogFile != NULL)
+		csvfilename = logfile_getname(fntime, ".csv");
 
 	/*
 	 * Decide whether to overwrite or append.  We can overwrite if (a)
@@ -1084,7 +1080,9 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 				Log_RotationAge = 0;
 				Log_RotationSize = 0;
 			}
-			pfree(filename);
+
+			if (filename)
+				pfree(filename);
 			if (csvfilename)
 				pfree(csvfilename);
 			return;
@@ -1100,8 +1098,10 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 #ifdef WIN32
 		EnterCriticalSection(&sysfileSection);
 #endif
+
 		fclose(syslogFile);
 		syslogFile = fh;
+
 #ifdef WIN32
 		LeaveCriticalSection(&sysfileSection);
 #endif
@@ -1110,6 +1110,7 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 		if (last_file_name != NULL)
 			pfree(last_file_name);
 		last_file_name = filename;
+		filename = NULL;
 	}
 
 	/* Same as above, but for csv file. */
@@ -1146,7 +1147,11 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 				Log_RotationAge = 0;
 				Log_RotationSize = 0;
 			}
-			pfree(csvfilename);
+
+			if (filename)
+				pfree(filename);
+			if (csvfilename)
+				pfree(csvfilename);
 			return;
 		}
 
@@ -1160,8 +1165,10 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 #ifdef WIN32
 		EnterCriticalSection(&sysfileSection);
 #endif
+
 		fclose(csvlogFile);
 		csvlogFile = fh;
+
 #ifdef WIN32
 		LeaveCriticalSection(&sysfileSection);
 #endif
@@ -1170,8 +1177,14 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 		if (last_csv_file_name != NULL)
 			pfree(last_csv_file_name);
 		last_csv_file_name = csvfilename;
+		csvfilename = NULL;
 	}
 
+	if (filename)
+		pfree(filename);
+	if (csvfilename)
+		pfree(csvfilename);
+
 	set_next_rotation_time();
 }
 
@@ -1179,10 +1192,13 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 /*
  * construct logfile name using timestamp information
  *
+ * If suffix isn't NULL, append it to the name, replacing any ".log"
+ * that may be in the pattern.
+ *
  * Result is palloc'd.
  */
 static char *
-logfile_getname(pg_time_t timestamp, char *suffix)
+logfile_getname(pg_time_t timestamp, const char *suffix)
 {
 	char	   *filename;
 	int			len;
@@ -1193,7 +1209,7 @@ logfile_getname(pg_time_t timestamp, char *suffix)
 
 	len = strlen(filename);
 
-	/* treat it as a strftime pattern */
+	/* treat Log_filename as a strftime pattern */
 	pg_strftime(filename + len, MAXPGPATH - len, Log_filename,
 				pg_localtime(&timestamp, log_timezone));
 
@@ -1202,7 +1218,7 @@ logfile_getname(pg_time_t timestamp, char *suffix)
 		len = strlen(filename);
 		if (len > 4 && (strcmp(filename + (len - 4), ".log") == 0))
 			len -= 4;
-		strncpy(filename + len, suffix, MAXPGPATH - len);
+		strlcpy(filename + len, suffix, MAXPGPATH - len);
 	}
 
 	return filename;
-- 
GitLab