Repair various defects in dc21234005.

pg_combinebackup had various problems:

* strncpy was used in various places where strlcpy should be used
  instead, to avoid any possibility of the result not being
  \0-terminated.
* scan_for_existing_tablespaces() failed to close the directory,
  and an error when opening the directory was reported with the
  wrong pathname.
* write_reconstructed_file() contained some redundant and therefore
  dead code.
* flush_manifest() didn't check the result of pg_checksum_update()
  as we do in other places, and misused a local pathname variable
  that shouldn't exist at all.

In pg_basebackup, the wrong variable name was used in one place,
due to a copy and paste that was not properly adjusted.

In blkreftable.c, the loop incorrectly doubled chunkno instead of
max_chunks. Fix that. Also remove a nearby assertion per repeated
off-list complaints from Tom Lane.

Per Coverity and subsequent code inspection by me and by Tom Lane.

Discussion: http://postgr.es/m/CA+Tgmobvqqj-DW9F7uUzT-cQqs6wcVb-Xhs=w=hzJnXSE-kRGw@mail.gmail.com
This commit is contained in:
Robert Haas 2024-01-11 13:06:10 -05:00
parent ee1bfd1683
commit 3d5c332a3d
5 changed files with 18 additions and 17 deletions

View File

@ -709,7 +709,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier,
PQserverVersion(conn) < MINIMUM_VERSION_FOR_PG_WAL ? PQserverVersion(conn) < MINIMUM_VERSION_FOR_PG_WAL ?
"pg_xlog" : "pg_wal"); "pg_xlog" : "pg_wal");
if (pg_mkdir_p(statusdir, pg_dir_create_mode) != 0 && if (pg_mkdir_p(summarydir, pg_dir_create_mode) != 0 &&
errno != EEXIST) errno != EEXIST)
pg_fatal("could not create directory \"%s\": %m", summarydir); pg_fatal("could not create directory \"%s\": %m", summarydir);
} }

View File

@ -773,8 +773,8 @@ process_directory_recursively(Oid tsoid,
*/ */
if (relative_path == NULL) if (relative_path == NULL)
{ {
strncpy(ifulldir, input_directory, MAXPGPATH); strlcpy(ifulldir, input_directory, MAXPGPATH);
strncpy(ofulldir, output_directory, MAXPGPATH); strlcpy(ofulldir, output_directory, MAXPGPATH);
if (OidIsValid(tsoid)) if (OidIsValid(tsoid))
snprintf(manifest_prefix, MAXPGPATH, "pg_tblspc/%u/", tsoid); snprintf(manifest_prefix, MAXPGPATH, "pg_tblspc/%u/", tsoid);
else else
@ -855,7 +855,7 @@ process_directory_recursively(Oid tsoid,
/* Append new pathname component to relative path. */ /* Append new pathname component to relative path. */
if (relative_path == NULL) if (relative_path == NULL)
strncpy(new_relative_path, de->d_name, MAXPGPATH); strlcpy(new_relative_path, de->d_name, MAXPGPATH);
else else
snprintf(new_relative_path, MAXPGPATH, "%s/%s", relative_path, snprintf(new_relative_path, MAXPGPATH, "%s/%s", relative_path,
de->d_name); de->d_name);
@ -1131,7 +1131,7 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
pg_log_debug("scanning \"%s\"", pg_tblspc); pg_log_debug("scanning \"%s\"", pg_tblspc);
if ((dir = opendir(pg_tblspc)) == NULL) if ((dir = opendir(pg_tblspc)) == NULL)
pg_fatal("could not open directory \"%s\": %m", pathname); pg_fatal("could not open directory \"%s\": %m", pg_tblspc);
while (errno = 0, (de = readdir(dir)) != NULL) while (errno = 0, (de = readdir(dir)) != NULL)
{ {
@ -1203,8 +1203,8 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
{ {
if (strcmp(tsmap->old_dir, link_target) == 0) if (strcmp(tsmap->old_dir, link_target) == 0)
{ {
strncpy(ts->old_dir, tsmap->old_dir, MAXPGPATH); strlcpy(ts->old_dir, tsmap->old_dir, MAXPGPATH);
strncpy(ts->new_dir, tsmap->new_dir, MAXPGPATH); strlcpy(ts->new_dir, tsmap->new_dir, MAXPGPATH);
ts->in_place = false; ts->in_place = false;
break; break;
} }
@ -1238,6 +1238,9 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
tslist = ts; tslist = ts;
} }
if (closedir(dir) != 0)
pg_fatal("could not close directory \"%s\": %m", pg_tblspc);
return tslist; return tslist;
} }

View File

@ -577,13 +577,12 @@ write_reconstructed_file(char *input_filename,
{ {
if (current_block == start_of_range) if (current_block == start_of_range)
appendStringInfo(&debug_buf, " %u:%s@" UINT64_FORMAT, appendStringInfo(&debug_buf, " %u:%s@" UINT64_FORMAT,
current_block, current_block, s->filename,
s == NULL ? "ZERO" : s->filename,
(uint64) offsetmap[current_block]); (uint64) offsetmap[current_block]);
else else
appendStringInfo(&debug_buf, " %u-%u:%s@" UINT64_FORMAT, appendStringInfo(&debug_buf, " %u-%u:%s@" UINT64_FORMAT,
start_of_range, current_block, start_of_range, current_block,
s == NULL ? "ZERO" : s->filename, s->filename,
(uint64) offsetmap[current_block]); (uint64) offsetmap[current_block]);
} }

View File

@ -241,8 +241,6 @@ escape_json(StringInfo buf, const char *str)
static void static void
flush_manifest(manifest_writer *mwriter) flush_manifest(manifest_writer *mwriter)
{ {
char pathname[MAXPGPATH];
if (mwriter->fd == -1 && if (mwriter->fd == -1 &&
(mwriter->fd = open(mwriter->pathname, (mwriter->fd = open(mwriter->pathname,
O_WRONLY | O_CREAT | O_EXCL | PG_BINARY, O_WRONLY | O_CREAT | O_EXCL | PG_BINARY,
@ -260,13 +258,15 @@ flush_manifest(manifest_writer *mwriter)
pg_fatal("could not write \"%s\": %m", mwriter->pathname); pg_fatal("could not write \"%s\": %m", mwriter->pathname);
else else
pg_fatal("could not write file \"%s\": wrote only %d of %d bytes", pg_fatal("could not write file \"%s\": wrote only %d of %d bytes",
pathname, (int) wb, mwriter->buf.len); mwriter->pathname, (int) wb, mwriter->buf.len);
} }
if (mwriter->still_checksumming) if (mwriter->still_checksumming &&
pg_checksum_update(&mwriter->manifest_ctx, pg_checksum_update(&mwriter->manifest_ctx,
(uint8 *) mwriter->buf.data, (uint8 *) mwriter->buf.data,
mwriter->buf.len); mwriter->buf.len) < 0)
pg_fatal("could not update checksum of file \"%s\"",
mwriter->pathname);
resetStringInfo(&mwriter->buf); resetStringInfo(&mwriter->buf);
} }
} }

View File

@ -988,8 +988,7 @@ BlockRefTableEntryMarkBlockModified(BlockRefTableEntry *entry,
*/ */
max_chunks = Max(16, entry->nchunks); max_chunks = Max(16, entry->nchunks);
while (max_chunks < chunkno + 1) while (max_chunks < chunkno + 1)
chunkno *= 2; max_chunks *= 2;
Assert(max_chunks > chunkno);
extra_chunks = max_chunks - entry->nchunks; extra_chunks = max_chunks - entry->nchunks;
if (entry->nchunks == 0) if (entry->nchunks == 0)