From 8a2822f52faa46955be97340741310327351374f Mon Sep 17 00:00:00 2001 From: David Steele Date: Fri, 23 Oct 2026 12:33:19 +0000 Subject: Avoid a torn copy of pg_control in base backups. Base backups read pg_control from disk, which can return a torn copy if the server is rewriting the file at the same time. 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. Commit 63a582222c6 dealt with the same hazard in frontend tools by retrying the read until the CRC is stable, which is appropriate for programs that have not access to shared memory. Commit c558e6fd92f made the SQL functions take ControlFileLock while reading the file. A base backup runs in the server, which already has the contents of pg_control in shared memory, so instead copy them from there under ControlFileLock. No read of the file is needed at all, and the lock is held only for the copy. The CRC is recalculated because the in-memory copy can be updated without being written, e.g. during replay of an online checkpoint on a standby, so the CRC it holds cannot be relied on. Backups taken with the low-level API still copy pg_control from disk and are not helped by this change. --- src/backend/access/transam/xlog.c | 36 +++++++++++++++++++++++++++++++ src/backend/backup/basebackup.c | 15 ++++++------- src/include/access/xlog.h | 1 + 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 8636ad9ab2c..e89227ed8b9 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -10608,6 +10608,42 @@ do_pg_abort_backup(int code, Datum arg) } } +/* + * Create a consistent copy of control data to be used for backup. + * + * Reading pg_control from disk while it is being rewritten can return a torn + * copy, so make the copy from shared memory under ControlFileLock instead. + * + * The caller's buffer must be at least PG_CONTROL_FILE_SIZE bytes and is + * zero-padded to that size. + * + * All field access is done through a local, properly-aligned ControlFileData; + * the caller's buffer is only ever written via memcpy() and so need not be + * suitably aligned for ControlFileData. + */ +void +backup_control_file(uint8 *controlFile) +{ + ControlFileData controlData; + + LWLockAcquire(ControlFileLock, LW_SHARED); + memcpy(&controlData, ControlFile, sizeof(ControlFileData)); + LWLockRelease(ControlFileLock); + + /* + * Recalculate the CRC. The in-memory copy can be updated without being + * written, e.g. during replay of an online checkpoint, in which case its + * CRC does not match its contents. + */ + INIT_CRC32C(controlData.crc); + COMP_CRC32C(controlData.crc, &controlData, offsetof(ControlFileData, crc)); + FIN_CRC32C(controlData.crc); + + /* Copy into the caller's buffer, zero-padded to the full file size */ + memset(controlFile, 0, PG_CONTROL_FILE_SIZE); + memcpy(controlFile, &controlData, sizeof(ControlFileData)); +} + /* * Register a handler that will warn about unterminated backups at end of * session, unless this has already been done. diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index d7c485dc002..c9ee9e2cac8 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -23,6 +23,7 @@ #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" @@ -340,9 +341,9 @@ perform_base_backup(basebackup_options *opt, bbsink *sink, if (ti->path == NULL) { - struct stat statbuf; bool sendtblspclinks = true; char *backup_label; + uint8 controlFile[PG_CONTROL_FILE_SIZE]; bbsink_begin_archive(sink, "base.tar"); @@ -365,14 +366,10 @@ perform_base_backup(basebackup_options *opt, bbsink *sink, sendtblspclinks, &manifest, InvalidOid, ib); /* ... and pg_control after everything else. */ - if (lstat(XLOG_CONTROL_FILE, &statbuf) != 0) - 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); + backup_control_file(controlFile); + sendFileWithContent(sink, XLOG_CONTROL_FILE, + (char *) controlFile, PG_CONTROL_FILE_SIZE, + &manifest); } else { diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 2bf5714b51e..a2a640178c4 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -343,6 +343,7 @@ extern void do_pg_backup_start(const char *backupidstr, bool fast, StringInfo tblspcmapfile); extern void do_pg_backup_stop(BackupState *state, bool waitforarchive); extern void do_pg_abort_backup(int code, Datum arg); +extern void backup_control_file(uint8 *controlFile); extern void register_persistent_abort_backup_handler(void); extern SessionBackupState get_backup_status(void); -- 2.34.1