Fix a couple of memory leaks in src/bin/pg_basebackup/

michaelpq · michaelpq · commit c4ef3b81b854 · 2021-07-26T11:14:14.000+09:00
These have been introduced by 7fbe0c8, and could happen for pg_basebackup and pg_receivewal. Per report from Coverity for the ones in walmethods.c, I have spotted the ones in receivelog.c after more review. Backpatch-through: 10
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
@@ -121,6 +121,7 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
 		{
 			pg_log_error("could not get size of write-ahead log file \"%s\": %s",
 						 fn, stream->walmethod->getlasterror());
+			pg_free(fn);
 			return false;
 		}
 		if (size == WalSegSz)
@@ -131,6 +132,7 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
 			{
 				pg_log_error("could not open existing write-ahead log file \"%s\": %s",
 							 fn, stream->walmethod->getlasterror());
+				pg_free(fn);
 				return false;
 			}
 
@@ -139,11 +141,13 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
 			{
 				pg_log_error("could not fsync existing write-ahead log file \"%s\": %s",
 							 fn, stream->walmethod->getlasterror());
+				pg_free(fn);
 				stream->walmethod->close(f, CLOSE_UNLINK);
 				return false;
 			}
 
 			walfile = f;
+			pg_free(fn);
 			return true;
 		}
 		if (size != 0)
@@ -155,6 +159,7 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
 								  "write-ahead log file \"%s\" has %d bytes, should be 0 or %d",
 								  size),
 						 fn, (int) size, WalSegSz);
+			pg_free(fn);
 			return false;
 		}
 		/* File existed and was empty, so fall through and open */
@@ -168,9 +173,11 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
 	{
 		pg_log_error("could not open write-ahead log file \"%s\": %s",
 					 fn, stream->walmethod->getlasterror());
+		pg_free(fn);
 		return false;
 	}
 
+	pg_free(fn);
 	walfile = f;
 	return true;
 }
diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
@@ -95,6 +95,7 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	filename = dir_get_file_name(pathname, temp_suffix);
 	snprintf(tmppath, sizeof(tmppath), "%s/%s",
 			 dir_data->basedir, filename);
+	pg_free(filename);
 
 	/*
 	 * Open a file for non-compressed as well as compressed files. Tracking
@@ -255,11 +256,13 @@ dir_close(Walfile f, WalCloseMethod method)
 			filename = dir_get_file_name(df->pathname, df->temp_suffix);
 			snprintf(tmppath, sizeof(tmppath), "%s/%s",
 					 dir_data->basedir, filename);
+			pg_free(filename);
 
 			/* permanent name, so no need for the prefix */
 			filename2 = dir_get_file_name(df->pathname, NULL);
 			snprintf(tmppath2, sizeof(tmppath2), "%s/%s",
 					 dir_data->basedir, filename2);
+			pg_free(filename2);
 			r = durable_rename(tmppath, tmppath2);
 		}
 		else if (method == CLOSE_UNLINK)
@@ -270,6 +273,7 @@ dir_close(Walfile f, WalCloseMethod method)
 			filename = dir_get_file_name(df->pathname, df->temp_suffix);
 			snprintf(tmppath, sizeof(tmppath), "%s/%s",
 					 dir_data->basedir, filename);
+			pg_free(filename);
 			r = unlink(tmppath);
 		}
 		else
@@ -626,11 +630,14 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	if (tarCreateHeader(tar_data->currentfile->header, tmppath, NULL, 0, S_IRUSR | S_IWUSR, 0, 0, time(NULL)) != TAR_OK)
 	{
 		pg_free(tar_data->currentfile);
+		pg_free(tmppath);
 		tar_data->currentfile = NULL;
 		tar_set_error("could not create tar header");
 		return NULL;
 	}
 
+	pg_free(tmppath);
+
 #ifdef HAVE_LIBZ
 	if (tar_data->compression)
 	{
diff --git a/src/bin/pg_basebackup/walmethods.h b/src/bin/pg_basebackup/walmethods.h
@@ -53,8 +53,8 @@ struct WalWriteMethod
 	ssize_t		(*get_file_size) (const char *pathname);
 
 	/*
-	 * Return the name of the current file to work on, without the base
-	 * directory.  This is useful for logging.
+	 * Return the name of the current file to work on in pg_malloc()'d string,
+	 * without the base directory.  This is useful for logging.
 	 */
 	char	   *(*get_file_name) (const char *pathname, const char *temp_suffix);
 

Original file line number	Diff line number	Diff line change
`@@ -121,6 +121,7 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)`
`121`	`121`	`{`
`122`	`122`	`pg_log_error("could not get size of write-ahead log file \"%s\": %s",`
`123`	`123`	`fn, stream->walmethod->getlasterror());`
	`124`	`+ pg_free(fn);`
`124`	`125`	`return false;`
`125`	`126`	`}`
`126`	`127`	`if (size == WalSegSz)`
`@@ -131,6 +132,7 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)`
`131`	`132`	`{`
`132`	`133`	`pg_log_error("could not open existing write-ahead log file \"%s\": %s",`
`133`	`134`	`fn, stream->walmethod->getlasterror());`
	`135`	`+ pg_free(fn);`
`134`	`136`	`return false;`
`135`	`137`	`}`
`136`	`138`
`@@ -139,11 +141,13 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)`
`139`	`141`	`{`
`140`	`142`	`pg_log_error("could not fsync existing write-ahead log file \"%s\": %s",`
`141`	`143`	`fn, stream->walmethod->getlasterror());`
	`144`	`+ pg_free(fn);`
`142`	`145`	`stream->walmethod->close(f, CLOSE_UNLINK);`
`143`	`146`	`return false;`
`144`	`147`	`}`
`145`	`148`
`146`	`149`	`walfile = f;`
	`150`	`+ pg_free(fn);`
`147`	`151`	`return true;`
`148`	`152`	`}`
`149`	`153`	`if (size != 0)`
`@@ -155,6 +159,7 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)`
`155`	`159`	`"write-ahead log file \"%s\" has %d bytes, should be 0 or %d",`
`156`	`160`	`size),`
`157`	`161`	`fn, (int) size, WalSegSz);`
	`162`	`+ pg_free(fn);`
`158`	`163`	`return false;`
`159`	`164`	`}`
`160`	`165`	`/* File existed and was empty, so fall through and open */`
`@@ -168,9 +173,11 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)`
`168`	`173`	`{`
`169`	`174`	`pg_log_error("could not open write-ahead log file \"%s\": %s",`
`170`	`175`	`fn, stream->walmethod->getlasterror());`
	`176`	`+ pg_free(fn);`
`171`	`177`	`return false;`
`172`	`178`	`}`
`173`	`179`
	`180`	`+ pg_free(fn);`
`174`	`181`	`walfile = f;`
`175`	`182`	`return true;`
`176`	`183`	`}`