From c0a88ffad9ee92757e6ef2e7a806b8d73b77da5f Mon Sep 17 00:00:00 2001 From: David Steele Date: Sat, 24 Oct 2026 15:59:45 +0000 Subject: Avoid a torn copy of pg_control in base backups. Base backups read pg_control from disk without any protection against concurrent writes by the server. At least ext4 and NTFS can expose partially written contents to a concurrent read and a backup containing a torn pg_control fails to start because the CRC does not match. Read pg_control while holding ControlFileLock, as commit c558e6fd92f did for the SQL functions that read pg_control. The file is read into memory and sent from there so the lock is not held while the contents are streamed to the client. Commit 63a582222c6 dealt with the same hazard in frontend tools by retrying the read until the CRC is stable, but a base backup runs in the server and can use ControlFileLock instead. Since the read is done under the lock, a CRC mismatch means that pg_control is actually corrupt so raise an error rather than include it in the backup. Backups taken with the low-level API still copy pg_control from disk and are not affected by this change. Back-patch to all supported branches. Before v17, sendFileWithContent() gets the length of the content with strlen(), which would truncate pg_control at its first zero byte. Commit 7606175991f, which added the length argument, must be back-patched first. It is confined to basebackup.c where the function is static. --- src/backend/backup/basebackup.c | 35 ++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index d7c485dc002..b5740f758dd 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -23,9 +23,11 @@ #include "backup/basebackup_incremental.h" #include "backup/basebackup_sink.h" #include "backup/basebackup_target.h" +#include "catalog/pg_control.h" #include "catalog/pg_tablespace_d.h" #include "commands/defrem.h" #include "common/compression.h" +#include "common/controldata_utils.h" #include "common/file_perm.h" #include "common/file_utils.h" #include "lib/stringinfo.h" @@ -42,6 +44,7 @@ #include "storage/checksum.h" #include "storage/dsm_impl.h" #include "storage/ipc.h" +#include "storage/lwlock.h" #include "storage/reinit.h" #include "utils/builtins.h" #include "utils/guc.h" @@ -340,9 +343,11 @@ perform_base_backup(basebackup_options *opt, bbsink *sink, if (ti->path == NULL) { - struct stat statbuf; bool sendtblspclinks = true; char *backup_label; + ControlFileData *controlData; + bool crc_ok; + char controlFile[PG_CONTROL_FILE_SIZE]; bbsink_begin_archive(sink, "base.tar"); @@ -364,15 +369,27 @@ perform_base_backup(basebackup_options *opt, bbsink *sink, sendDir(sink, ".", 1, false, state.tablespaces, sendtblspclinks, &manifest, InvalidOid, ib); - /* ... and pg_control after everything else. */ - if (lstat(XLOG_CONTROL_FILE, &statbuf) != 0) + /* + * ... and pg_control after everything else. Read it while + * holding ControlFileLock so that a concurrent write cannot + * produce a torn copy. + */ + LWLockAcquire(ControlFileLock, LW_SHARED); + controlData = get_controlfile(DataDir, &crc_ok); + LWLockRelease(ControlFileLock); + + if (!crc_ok) ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not stat file \"%s\": %m", - XLOG_CONTROL_FILE))); - sendFile(sink, XLOG_CONTROL_FILE, XLOG_CONTROL_FILE, &statbuf, - false, InvalidOid, InvalidOid, - InvalidRelFileNumber, 0, &manifest, 0, NULL, 0); + (errmsg("calculated CRC checksum does not match value stored in file"))); + + /* Send the full file, zero-padded as it is on disk */ + memset(controlFile, 0, PG_CONTROL_FILE_SIZE); + memcpy(controlFile, controlData, sizeof(ControlFileData)); + pfree(controlData); + + sendFileWithContent(sink, XLOG_CONTROL_FILE, + controlFile, PG_CONTROL_FILE_SIZE, + &manifest); } else { -- 2.34.1