From 51d7bf5edcaf4f8a95bb10f96b7af76d00a2f83f Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 27 Oct 2026 13:53:26 +0900 Subject: [PATCH] Avoid truncated or zero-padded copies of replaced files in backups Base backups call stat() on each file while scanning the data directory, and later send exactly that many bytes. If the file is replaced by for example a concurrent rename(), the backup contains the new file truncated or zero-padded to the size of the old one. This is harmless for relation files as WAL replay repairs them. However, it causes harm for non-relation files manipulated as temporary file renamed in place. Here is a list of non-relation files that use a temporary file followed by a final rename() (I may be missing some): * postgresql.auto.conf * pg_logical/replorigin_checkpoint (a damaged copy fails its CRC check and prevents startup) * pg_prewarm's autoprewarm.blocks * pg_stash_advice.tsv. When sending files that are not relation files, fstat() is called again on the opened file and the refreshed stats are used for the tar header and the backup manifest. Since the rename is atomic, the opened file is always complete. Relation files keep using the earlier stat, to avoid copying data that WAL replay would restore anyway (because that's cheaper). Back-patch down to v15. v14 is going to be EOL soon, so I have decided to leave it out of the picture (risk/reward thing). Author: David Steele Reviewed-by: Chao Li Reviewed-by: Michael Paquier Reviewed-by: Shihao Zhong Discussion: https://postgr.es/m/CAGRkXqS_On-pLgcv6xjH8mvGhdcPmfWtythW3r8pYKUG+hsiuQ@mail.gmail.com Backpatch-through: 15 --- src/backend/backup/basebackup.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index d7c485dc002..64e5ca1ff3e 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -93,7 +93,7 @@ static int64 sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeo backup_manifest_info *manifest, Oid spcoid, IncrementalBackupInfo *ib); static bool sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, - struct stat *statbuf, bool missing_ok, + const struct stat *statbuf, bool missing_ok, Oid dboid, Oid spcoid, RelFileNumber relfilenumber, unsigned segno, backup_manifest_info *manifest, @@ -1575,12 +1575,15 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly, * If the whole file is to be sent, then incremental_blocks should be NULL, * and num_incremental_blocks can have any value, as it will be ignored. * + * For non-relation files, the stat data of the opened file is used, not + * 'statbuf'. + * * Returns true if the file was successfully sent, false if 'missing_ok', * and the file did not exist. */ static bool sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, - struct stat *statbuf, bool missing_ok, Oid dboid, Oid spcoid, + const struct stat *statbuf, bool missing_ok, Oid dboid, Oid spcoid, RelFileNumber relfilenumber, unsigned segno, backup_manifest_info *manifest, unsigned num_incremental_blocks, BlockNumber *incremental_blocks, unsigned truncation_block_length) @@ -1593,6 +1596,7 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, bool verify_checksum = false; pg_checksum_context checksum_ctx; int ibindex = 0; + struct stat filestat; if (pg_checksum_init(&checksum_ctx, manifest->checksum_type) < 0) elog(ERROR, "could not initialize checksum of file \"%s\"", @@ -1608,7 +1612,18 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, errmsg("could not open file \"%s\": %m", readfilename))); } - _tarWriteHeader(sink, tarfilename, NULL, statbuf, false); + /* + * Non-relation files may be replaced by a rename(), making irrelevant the + * caller's stat data. Reuse fstat() on the opened file. + */ + filestat = *statbuf; + + if (!RelFileNumberIsValid(relfilenumber) && fstat(fd, &filestat) != 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not stat file \"%s\": %m", readfilename))); + + _tarWriteHeader(sink, tarfilename, NULL, &filestat, false); /* * Checksums are verified in multiples of BLCKSZ, so the buffer length @@ -1697,13 +1712,13 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, */ if (incremental_blocks == NULL) { - size_t remaining = statbuf->st_size - bytes_done; + size_t remaining = filestat.st_size - bytes_done; /* * If we've read the required number of bytes, then it's time to * stop. */ - if (bytes_done >= statbuf->st_size) + if (bytes_done >= filestat.st_size) break; /* @@ -1798,9 +1813,9 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, } /* If the file was truncated while we were sending it, pad it with zeros */ - while (bytes_done < statbuf->st_size) + while (bytes_done < filestat.st_size) { - size_t remaining = statbuf->st_size - bytes_done; + size_t remaining = filestat.st_size - bytes_done; size_t nbytes = Min(sink->bbs_buffer_length, remaining); MemSet(sink->bbs_buffer, 0, nbytes); @@ -1835,8 +1850,8 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, total_checksum_failures += checksum_failures; - AddFileToBackupManifest(manifest, spcoid, tarfilename, statbuf->st_size, - (pg_time_t) statbuf->st_mtime, &checksum_ctx); + AddFileToBackupManifest(manifest, spcoid, tarfilename, filestat.st_size, + (pg_time_t) filestat.st_mtime, &checksum_ctx); return true; }