database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-06 19:59:18 +03:00
Code

Avoid dangling-pointer usage in pg_basebackup progress reports.

Browse Source 
Ill-considered refactoring in 23a1c6578 led to progress_filename
sometimes pointing to data that had gone out of scope.  The most
bulletproof fix is to hang onto a copy of whatever's passed in.
Compared to the work spent elsewhere per file, that's not very
expensive, plus we can skip it except in verbose logging mode.

Per buildfarm.

Discussion: https://postgr.es/m/20220212211316.GK31460@telsasoft.com
This commit is contained in:
Tom Lane 2022-02-17 15:03:40 -05:00
parent 138c51b721
commit 62cb7427d1
1 changed files with 16 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/bin/pg_basebackup/pg_basebackup.c
View File

@ -164,7 +164,7 @@ static bool found_tablespace_dirs = false;
static uint64 totalsize_kb; static uint64 totalsize_kb;
static uint64 totaldone; static uint64 totaldone;
static int tablespacecount; static int tablespacecount;
static const char *progress_filename; static char *progress_filename = NULL;
/* Pipe to communicate with background wal receiver process */ /* Pipe to communicate with background wal receiver process */
#ifndef WIN32 #ifndef WIN32
@ -775,11 +775,22 @@ verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found)
/* /*
* Callback to update our notion of the current filename. * Callback to update our notion of the current filename.
*
* No other code should modify progress_filename!
*/ */
static void static void
progress_update_filename(const char *filename) progress_update_filename(const char *filename)
{ {
progress_filename = filename; /* We needn't maintain this variable if not doing verbose reports. */
if (showprogress && verbose)
{
if (progress_filename)
free(progress_filename);
if (filename)
progress_filename = pg_strdup(filename);
else
progress_filename = NULL;
}
} }
/* /*
@ -1258,7 +1269,7 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
*/ */
if (must_parse_archive) if (must_parse_archive)
streamer = bbstreamer_tar_archiver_new(streamer); streamer = bbstreamer_tar_archiver_new(streamer);
progress_filename = archive_filename; progress_update_filename(archive_filename);
} }
/* /*
@ -1662,7 +1673,7 @@ ReceiveTarFile(PGconn *conn, char *archive_name, char *spclocation,
expect_unterminated_tarfile); expect_unterminated_tarfile);
state.tablespacenum = tablespacenum; state.tablespacenum = tablespacenum;
ReceiveCopyData(conn, ReceiveTarCopyChunk, &state); ReceiveCopyData(conn, ReceiveTarCopyChunk, &state);
progress_filename = NULL; progress_update_filename(NULL);
/* /*
* The decision as to whether we need to inject the backup manifest into * The decision as to whether we need to inject the backup manifest into
@ -2161,7 +2172,7 @@ BaseBackup(void)
if (showprogress) if (showprogress)
{ {
progress_filename = NULL; progress_update_filename(NULL);
progress_report(PQntuples(res), true, true); progress_report(PQntuples(res), true, true);
} }