diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 5b6cec8deed..3654543919f 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -4376,7 +4376,16 @@ static void WriteControlFile(void) { int fd; - char buffer[PG_CONTROL_SIZE]; /* need not be aligned */ + char buffer[PG_CONTROL_FILE_SIZE]; /* need not be aligned */ + + /* + * Ensure that the size of the pg_control data structure is sane. See the + * comments for these symbols in pg_control.h. + */ + StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE, + "pg_control is too large for atomic disk writes"); + StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE, + "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE"); /* * Initialize version and compatibility-check fields @@ -4409,16 +4418,13 @@ WriteControlFile(void) FIN_CRC32C(ControlFile->crc); /* - * We write out PG_CONTROL_SIZE bytes into pg_control, zero-padding the - * excess over sizeof(ControlFileData). This reduces the odds of + * We write out PG_CONTROL_FILE_SIZE bytes into pg_control, zero-padding + * the excess over sizeof(ControlFileData). This reduces the odds of * premature-EOF errors when reading pg_control. We'll still fail when we * check the contents of the file, but hopefully with a more specific * error than "couldn't read pg_control". */ - if (sizeof(ControlFileData) > PG_CONTROL_SIZE) - elog(PANIC, "sizeof(ControlFileData) is larger than PG_CONTROL_SIZE; fix either one"); - - memset(buffer, 0, PG_CONTROL_SIZE); + memset(buffer, 0, PG_CONTROL_FILE_SIZE); memcpy(buffer, ControlFile, sizeof(ControlFileData)); fd = BasicOpenFile(XLOG_CONTROL_FILE, @@ -4432,7 +4438,7 @@ WriteControlFile(void) errno = 0; pgstat_report_wait_start(WAIT_EVENT_CONTROL_FILE_WRITE); - if (write(fd, buffer, PG_CONTROL_SIZE) != PG_CONTROL_SIZE) + if (write(fd, buffer, PG_CONTROL_FILE_SIZE) != PG_CONTROL_FILE_SIZE) { /* if write didn't set errno, assume problem is no disk space */ if (errno == 0) diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index d4dd1d49ca5..ac678317795 100644 --- a/src/bin/pg_resetwal/pg_resetwal.c +++ b/src/bin/pg_resetwal/pg_resetwal.c @@ -552,9 +552,9 @@ ReadControlFile(void) } /* Use malloc to ensure we have a maxaligned buffer */ - buffer = (char *) pg_malloc(PG_CONTROL_SIZE); + buffer = (char *) pg_malloc(PG_CONTROL_FILE_SIZE); - len = read(fd, buffer, PG_CONTROL_SIZE); + len = read(fd, buffer, PG_CONTROL_FILE_SIZE); if (len < 0) { fprintf(stderr, _("%s: could not read file \"%s\": %s\n"), @@ -834,7 +834,16 @@ static void RewriteControlFile(void) { int fd; - char buffer[PG_CONTROL_SIZE]; /* need not be aligned */ + char buffer[PG_CONTROL_FILE_SIZE]; /* need not be aligned */ + + /* + * For good luck, apply the same static assertions as in backend's + * WriteControlFile(). + */ + StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE, + "pg_control is too large for atomic disk writes"); + StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE, + "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE"); /* * Adjust fields as needed to force an empty XLOG starting at @@ -878,21 +887,13 @@ RewriteControlFile(void) FIN_CRC32C(ControlFile.crc); /* - * We write out PG_CONTROL_SIZE bytes into pg_control, zero-padding the - * excess over sizeof(ControlFileData). This reduces the odds of + * We write out PG_CONTROL_FILE_SIZE bytes into pg_control, zero-padding + * the excess over sizeof(ControlFileData). This reduces the odds of * premature-EOF errors when reading pg_control. We'll still fail when we * check the contents of the file, but hopefully with a more specific * error than "couldn't read pg_control". */ - if (sizeof(ControlFileData) > PG_CONTROL_SIZE) - { - fprintf(stderr, - _("%s: internal error -- sizeof(ControlFileData) is too large ... fix PG_CONTROL_SIZE\n"), - progname); - exit(1); - } - - memset(buffer, 0, PG_CONTROL_SIZE); + memset(buffer, 0, PG_CONTROL_FILE_SIZE); memcpy(buffer, &ControlFile, sizeof(ControlFileData)); unlink(XLOG_CONTROL_FILE); @@ -908,7 +909,7 @@ RewriteControlFile(void) } errno = 0; - if (write(fd, buffer, PG_CONTROL_SIZE) != PG_CONTROL_SIZE) + if (write(fd, buffer, PG_CONTROL_FILE_SIZE) != PG_CONTROL_FILE_SIZE) { /* if write didn't set errno, assume problem is no disk space */ if (errno == 0) diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 6c75b56992a..4bd1a759734 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -625,9 +625,9 @@ checkControlFile(ControlFileData *ControlFile) static void digestControlFile(ControlFileData *ControlFile, char *src, size_t size) { - if (size != PG_CONTROL_SIZE) + if (size != PG_CONTROL_FILE_SIZE) pg_fatal("unexpected control file size %d, expected %d\n", - (int) size, PG_CONTROL_SIZE); + (int) size, PG_CONTROL_FILE_SIZE); memcpy(ControlFile, src, sizeof(ControlFileData)); @@ -641,7 +641,16 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size) static void updateControlFile(ControlFileData *ControlFile) { - char buffer[PG_CONTROL_SIZE]; + char buffer[PG_CONTROL_FILE_SIZE]; + + /* + * For good luck, apply the same static assertions as in backend's + * WriteControlFile(). + */ + StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE, + "pg_control is too large for atomic disk writes"); + StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE, + "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE"); /* Recalculate CRC of control file */ INIT_CRC32C(ControlFile->crc); @@ -651,16 +660,16 @@ updateControlFile(ControlFileData *ControlFile) FIN_CRC32C(ControlFile->crc); /* - * Write out PG_CONTROL_SIZE bytes into pg_control by zero-padding the - * excess over sizeof(ControlFileData) to avoid premature EOF related + * Write out PG_CONTROL_FILE_SIZE bytes into pg_control by zero-padding + * the excess over sizeof(ControlFileData), to avoid premature EOF related * errors when reading it. */ - memset(buffer, 0, PG_CONTROL_SIZE); + memset(buffer, 0, PG_CONTROL_FILE_SIZE); memcpy(buffer, ControlFile, sizeof(ControlFileData)); open_target_file("global/pg_control", false); - write_target_range(buffer, 0, PG_CONTROL_SIZE); + write_target_range(buffer, 0, PG_CONTROL_FILE_SIZE); close_target_file(); } diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h index 84327c9da6b..1ec03caf5fb 100644 --- a/src/include/catalog/pg_control.h +++ b/src/include/catalog/pg_control.h @@ -20,11 +20,12 @@ #include "port/pg_crc32c.h" -#define MOCK_AUTH_NONCE_LEN 32 - /* Version identifier for this pg_control format */ #define PG_CONTROL_VERSION 1002 +/* Nonce key length, see below */ +#define MOCK_AUTH_NONCE_LEN 32 + /* * Body of CheckPoint XLOG records. This is declared here because we keep * a copy of the latest one in pg_control for possible disaster recovery. @@ -94,10 +95,6 @@ typedef enum DBState /* * Contents of pg_control. - * - * NOTE: try to keep this under 512 bytes so that it will fit on one physical - * sector of typical disk drives. This reduces the odds of corruption due to - * power failure midway through a write. */ typedef struct ControlFileData @@ -235,6 +232,14 @@ typedef struct ControlFileData pg_crc32c crc; } ControlFileData; +/* + * Maximum safe value of sizeof(ControlFileData). For reliability's sake, + * it's critical that pg_control updates be atomic writes. That generally + * means the active data can't be more than one disk sector, which is 512 + * bytes on common hardware. Be very careful about raising this limit. + */ +#define PG_CONTROL_MAX_SAFE_SIZE 512 + /* * Physical size of the pg_control file. Note that this is considerably * bigger than the actually used size (ie, sizeof(ControlFileData)). @@ -242,6 +247,6 @@ typedef struct ControlFileData * changes, so that ReadControlFile will deliver a suitable wrong-version * message instead of a read error if it's looking at an incompatible file. */ -#define PG_CONTROL_SIZE 8192 +#define PG_CONTROL_FILE_SIZE 8192 #endif /* PG_CONTROL_H */