Skip to content

Commit 7b9e2c4

Browse files
dwsteeleCommitfest Bot
authored andcommitted
Add pg_control flag to prevent recovery without backup_label.
Harden recovery by adding a flag to pg_control to indicate that backup_label is required. This prevents the user from deleting backup_label resulting in an inconsistent recovery. Another advantage is that the copy of pg_control used by pg_basebackup is guaranteed not to be torn. This functionality is limited to pg_basebackup (or any software comfortable with modifying pg_control). Control and catalog version bumps are required.
1 parent 2648eab commit 7b9e2c4

File tree

12 files changed

+80
-15
lines changed

12 files changed

+80
-15
lines changed

doc/src/sgml/func/func-info.sgml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3598,6 +3598,11 @@ acl | {postgres=arwdDxtm/postgres,foo=r/postgres}
35983598
<entry><type>boolean</type></entry>
35993599
</row>
36003600

3601+
<row>
3602+
<entry><structfield>backup_label_required</structfield></entry>
3603+
<entry><type>boolean</type></entry>
3604+
</row>
3605+
36013606
</tbody>
36023607
</tgroup>
36033608
</table>

src/backend/access/transam/xlog.c

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9459,6 +9459,29 @@ do_pg_abort_backup(int code, Datum arg)
94599459
}
94609460
}
94619461

9462+
/*
9463+
* Create a consistent copy of control data to be used for backup and update it
9464+
* to require a backup label for recovery. Also recalculate the CRC.
9465+
*/
9466+
void
9467+
backup_control_file(uint8_t *controlFile)
9468+
{
9469+
ControlFileData *controlData = ((ControlFileData *)controlFile);
9470+
9471+
memset(controlFile + sizeof(ControlFileData), 0,
9472+
PG_CONTROL_FILE_SIZE - sizeof(ControlFileData));
9473+
9474+
LWLockAcquire(ControlFileLock, LW_SHARED);
9475+
memcpy(controlFile, ControlFile, sizeof(ControlFileData));
9476+
LWLockRelease(ControlFileLock);
9477+
9478+
controlData->backupLabelRequired = true;
9479+
9480+
INIT_CRC32C(controlData->crc);
9481+
COMP_CRC32C(controlData->crc, controlFile, offsetof(ControlFileData, crc));
9482+
FIN_CRC32C(controlData->crc);
9483+
}
9484+
94629485
/*
94639486
* Register a handler that will warn about unterminated backups at end of
94649487
* session, unless this has already been done.

src/backend/access/transam/xlogrecovery.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -710,7 +710,14 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
710710
}
711711
else
712712
{
713-
/* No backup_label file has been found if we are here. */
713+
/*
714+
* No backup_label file has been found if we are here. Error if the
715+
* control file requires backup_label.
716+
*/
717+
if (ControlFile->backupLabelRequired)
718+
ereport(FATAL,
719+
(errmsg("could not find backup_label required for recovery"),
720+
errhint("backup_label must be present for recovery to proceed")));
714721

715722
/*
716723
* If tablespace_map file is present without backup_label file, there
@@ -985,6 +992,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
985992
{
986993
ControlFile->backupStartPoint = checkPoint.redo;
987994
ControlFile->backupEndRequired = backupEndRequired;
995+
ControlFile->backupLabelRequired = false;
988996

989997
if (backupFromStandby)
990998
{

src/backend/backup/basebackup.c

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "backup/basebackup_incremental.h"
2424
#include "backup/basebackup_sink.h"
2525
#include "backup/basebackup_target.h"
26+
#include "catalog/pg_control.h"
2627
#include "catalog/pg_tablespace_d.h"
2728
#include "commands/defrem.h"
2829
#include "common/compression.h"
@@ -326,9 +327,9 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
326327

327328
if (ti->path == NULL)
328329
{
329-
struct stat statbuf;
330330
bool sendtblspclinks = true;
331331
char *backup_label;
332+
uint8_t controlFile[PG_CONTROL_FILE_SIZE];
332333

333334
bbsink_begin_archive(sink, "base.tar");
334335

@@ -351,14 +352,10 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
351352
sendtblspclinks, &manifest, InvalidOid, ib);
352353

353354
/* ... and pg_control after everything else. */
354-
if (lstat(XLOG_CONTROL_FILE, &statbuf) != 0)
355-
ereport(ERROR,
356-
(errcode_for_file_access(),
357-
errmsg("could not stat file \"%s\": %m",
358-
XLOG_CONTROL_FILE)));
359-
sendFile(sink, XLOG_CONTROL_FILE, XLOG_CONTROL_FILE, &statbuf,
360-
false, InvalidOid, InvalidOid,
361-
InvalidRelFileNumber, 0, &manifest, 0, NULL, 0);
355+
backup_control_file(controlFile);
356+
sendFileWithContent(sink, XLOG_CONTROL_FILE,
357+
(char *)controlFile, PG_CONTROL_FILE_SIZE,
358+
&manifest);
362359
}
363360
else
364361
{

src/backend/utils/misc/pg_controldata.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,8 @@ pg_control_checkpoint(PG_FUNCTION_ARGS)
162162
Datum
163163
pg_control_recovery(PG_FUNCTION_ARGS)
164164
{
165-
Datum values[5];
166-
bool nulls[5];
165+
Datum values[6];
166+
bool nulls[6];
167167
TupleDesc tupdesc;
168168
HeapTuple htup;
169169
ControlFileData *ControlFile;
@@ -195,6 +195,9 @@ pg_control_recovery(PG_FUNCTION_ARGS)
195195
values[4] = BoolGetDatum(ControlFile->backupEndRequired);
196196
nulls[4] = false;
197197

198+
values[5] = BoolGetDatum(ControlFile->backupLabelRequired);
199+
nulls[5] = false;
200+
198201
htup = heap_form_tuple(tupdesc, values, nulls);
199202

200203
PG_RETURN_DATUM(HeapTupleGetDatum(htup));

src/bin/pg_controldata/pg_controldata.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,8 @@ main(int argc, char *argv[])
294294
LSN_FORMAT_ARGS(ControlFile->backupEndPoint));
295295
printf(_("End-of-backup record required: %s\n"),
296296
ControlFile->backupEndRequired ? _("yes") : _("no"));
297+
printf(_("Backup label required: %s\n"),
298+
ControlFile->backupLabelRequired ? _("yes") : _("no"));
297299
printf(_("wal_level setting: %s\n"),
298300
wal_level_str(ControlFile->wal_level));
299301
printf(_("wal_log_hints setting: %s\n"),

src/bin/pg_resetwal/pg_resetwal.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,7 @@ RewriteControlFile(void)
883883
ControlFile.backupStartPoint = 0;
884884
ControlFile.backupEndPoint = 0;
885885
ControlFile.backupEndRequired = false;
886+
ControlFile.backupLabelRequired = false;
886887

887888
/*
888889
* Force the defaults for max_* settings. The values don't really matter

src/bin/pg_rewind/pg_rewind.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,7 @@ perform_rewind(filemap_t *filemap, rewind_source *source,
734734
ControlFile_new.minRecoveryPoint = endrec;
735735
ControlFile_new.minRecoveryPointTLI = endtli;
736736
ControlFile_new.state = DB_IN_ARCHIVE_RECOVERY;
737+
ControlFile_new.backupLabelRequired = true;
737738
if (!dry_run)
738739
update_controlfile(datadir_target, &ControlFile_new, do_sync);
739740
}

src/include/access/xlog.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ extern void do_pg_backup_start(const char *backupidstr, bool fast,
296296
StringInfo tblspcmapfile);
297297
extern void do_pg_backup_stop(BackupState *state, bool waitforarchive);
298298
extern void do_pg_abort_backup(int code, Datum arg);
299+
extern void backup_control_file(uint8_t *controlFile);
299300
extern void register_persistent_abort_backup_handler(void);
300301
extern SessionBackupState get_backup_status(void);
301302

src/include/catalog/pg_control.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,16 @@ typedef struct ControlFileData
164164
* If backupEndRequired is true, we know for sure that we're restoring
165165
* from a backup, and must see a backup-end record before we can safely
166166
* start up.
167+
*
168+
* If backupLabelRequired is true, then a backup_label file must be
169+
* present in order for recovery to proceed.
167170
*/
168171
XLogRecPtr minRecoveryPoint;
169172
TimeLineID minRecoveryPointTLI;
170173
XLogRecPtr backupStartPoint;
171174
XLogRecPtr backupEndPoint;
172175
bool backupEndRequired;
176+
bool backupLabelRequired;
173177

174178
/*
175179
* Parameter settings that determine if the WAL can be used for archival

0 commit comments

Comments
 (0)