Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit3cb29c4

Browse files
committed
Add static assertions about pg_control fitting into one disk sector.
When pg_control was first designed, sizeof(ControlFileData) was smallenough that a comment seemed like plenty to document the assumption thatit'd fit into one disk sector. Now it's nearly 300 bytes, raising thepossibility that somebody would carelessly add enough stuff to createa problem. Let's add a StaticAssertStmt() to ensure that the situationdoesn't pass unnoticed if it ever occurs.While at it, rename PG_CONTROL_SIZE to PG_CONTROL_FILE_SIZE to make itclearer what that symbol means, and convert the existing runtimecomparisons of sizeof(ControlFileData) vs. PG_CONTROL_FILE_SIZE to bestatic asserts --- we didn't have that technology when this code wasfirst written.Discussion:https://postgr.es/m/9192.1500490591@sss.pgh.pa.us
1 parent5752dcd commit3cb29c4

File tree

4 files changed

+58
-37
lines changed

4 files changed

+58
-37
lines changed

‎src/backend/access/transam/xlog.c

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4376,7 +4376,16 @@ static void
43764376
WriteControlFile(void)
43774377
{
43784378
intfd;
4379-
charbuffer[PG_CONTROL_SIZE];/* need not be aligned */
4379+
charbuffer[PG_CONTROL_FILE_SIZE];/* need not be aligned */
4380+
4381+
/*
4382+
* Ensure that the size of the pg_control data structure is sane. See the
4383+
* comments for these symbols in pg_control.h.
4384+
*/
4385+
StaticAssertStmt(sizeof(ControlFileData) <=PG_CONTROL_MAX_SAFE_SIZE,
4386+
"pg_control is too large for atomic disk writes");
4387+
StaticAssertStmt(sizeof(ControlFileData) <=PG_CONTROL_FILE_SIZE,
4388+
"sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE");
43804389

43814390
/*
43824391
* Initialize version and compatibility-check fields
@@ -4409,16 +4418,13 @@ WriteControlFile(void)
44094418
FIN_CRC32C(ControlFile->crc);
44104419

44114420
/*
4412-
* We write outPG_CONTROL_SIZE bytes into pg_control, zero-padding the
4413-
* excess over sizeof(ControlFileData). This reduces the odds of
4421+
* We write outPG_CONTROL_FILE_SIZE bytes into pg_control, zero-padding
4422+
*theexcess over sizeof(ControlFileData). This reduces the odds of
44144423
* premature-EOF errors when reading pg_control. We'll still fail when we
44154424
* check the contents of the file, but hopefully with a more specific
44164425
* error than "couldn't read pg_control".
44174426
*/
4418-
if (sizeof(ControlFileData)>PG_CONTROL_SIZE)
4419-
elog(PANIC,"sizeof(ControlFileData) is larger than PG_CONTROL_SIZE; fix either one");
4420-
4421-
memset(buffer,0,PG_CONTROL_SIZE);
4427+
memset(buffer,0,PG_CONTROL_FILE_SIZE);
44224428
memcpy(buffer,ControlFile,sizeof(ControlFileData));
44234429

44244430
fd=BasicOpenFile(XLOG_CONTROL_FILE,
@@ -4432,7 +4438,7 @@ WriteControlFile(void)
44324438

44334439
errno=0;
44344440
pgstat_report_wait_start(WAIT_EVENT_CONTROL_FILE_WRITE);
4435-
if (write(fd,buffer,PG_CONTROL_SIZE)!=PG_CONTROL_SIZE)
4441+
if (write(fd,buffer,PG_CONTROL_FILE_SIZE)!=PG_CONTROL_FILE_SIZE)
44364442
{
44374443
/* if write didn't set errno, assume problem is no disk space */
44384444
if (errno==0)

‎src/bin/pg_resetwal/pg_resetwal.c

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -552,9 +552,9 @@ ReadControlFile(void)
552552
}
553553

554554
/* Use malloc to ensure we have a maxaligned buffer */
555-
buffer= (char*)pg_malloc(PG_CONTROL_SIZE);
555+
buffer= (char*)pg_malloc(PG_CONTROL_FILE_SIZE);
556556

557-
len=read(fd,buffer,PG_CONTROL_SIZE);
557+
len=read(fd,buffer,PG_CONTROL_FILE_SIZE);
558558
if (len<0)
559559
{
560560
fprintf(stderr,_("%s: could not read file \"%s\": %s\n"),
@@ -834,7 +834,16 @@ static void
834834
RewriteControlFile(void)
835835
{
836836
intfd;
837-
charbuffer[PG_CONTROL_SIZE];/* need not be aligned */
837+
charbuffer[PG_CONTROL_FILE_SIZE];/* need not be aligned */
838+
839+
/*
840+
* For good luck, apply the same static assertions as in backend's
841+
* WriteControlFile().
842+
*/
843+
StaticAssertStmt(sizeof(ControlFileData) <=PG_CONTROL_MAX_SAFE_SIZE,
844+
"pg_control is too large for atomic disk writes");
845+
StaticAssertStmt(sizeof(ControlFileData) <=PG_CONTROL_FILE_SIZE,
846+
"sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE");
838847

839848
/*
840849
* Adjust fields as needed to force an empty XLOG starting at
@@ -878,21 +887,13 @@ RewriteControlFile(void)
878887
FIN_CRC32C(ControlFile.crc);
879888

880889
/*
881-
* We write outPG_CONTROL_SIZE bytes into pg_control, zero-padding the
882-
* excess over sizeof(ControlFileData). This reduces the odds of
890+
* We write outPG_CONTROL_FILE_SIZE bytes into pg_control, zero-padding
891+
*theexcess over sizeof(ControlFileData). This reduces the odds of
883892
* premature-EOF errors when reading pg_control. We'll still fail when we
884893
* check the contents of the file, but hopefully with a more specific
885894
* error than "couldn't read pg_control".
886895
*/
887-
if (sizeof(ControlFileData)>PG_CONTROL_SIZE)
888-
{
889-
fprintf(stderr,
890-
_("%s: internal error -- sizeof(ControlFileData) is too large ... fix PG_CONTROL_SIZE\n"),
891-
progname);
892-
exit(1);
893-
}
894-
895-
memset(buffer,0,PG_CONTROL_SIZE);
896+
memset(buffer,0,PG_CONTROL_FILE_SIZE);
896897
memcpy(buffer,&ControlFile,sizeof(ControlFileData));
897898

898899
unlink(XLOG_CONTROL_FILE);
@@ -908,7 +909,7 @@ RewriteControlFile(void)
908909
}
909910

910911
errno=0;
911-
if (write(fd,buffer,PG_CONTROL_SIZE)!=PG_CONTROL_SIZE)
912+
if (write(fd,buffer,PG_CONTROL_FILE_SIZE)!=PG_CONTROL_FILE_SIZE)
912913
{
913914
/* if write didn't set errno, assume problem is no disk space */
914915
if (errno==0)

‎src/bin/pg_rewind/pg_rewind.c

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -625,9 +625,9 @@ checkControlFile(ControlFileData *ControlFile)
625625
staticvoid
626626
digestControlFile(ControlFileData*ControlFile,char*src,size_tsize)
627627
{
628-
if (size!=PG_CONTROL_SIZE)
628+
if (size!=PG_CONTROL_FILE_SIZE)
629629
pg_fatal("unexpected control file size %d, expected %d\n",
630-
(int)size,PG_CONTROL_SIZE);
630+
(int)size,PG_CONTROL_FILE_SIZE);
631631

632632
memcpy(ControlFile,src,sizeof(ControlFileData));
633633

@@ -641,7 +641,16 @@ digestControlFile(ControlFileData *ControlFile, char *src, size_t size)
641641
staticvoid
642642
updateControlFile(ControlFileData*ControlFile)
643643
{
644-
charbuffer[PG_CONTROL_SIZE];
644+
charbuffer[PG_CONTROL_FILE_SIZE];
645+
646+
/*
647+
* For good luck, apply the same static assertions as in backend's
648+
* WriteControlFile().
649+
*/
650+
StaticAssertStmt(sizeof(ControlFileData) <=PG_CONTROL_MAX_SAFE_SIZE,
651+
"pg_control is too large for atomic disk writes");
652+
StaticAssertStmt(sizeof(ControlFileData) <=PG_CONTROL_FILE_SIZE,
653+
"sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE");
645654

646655
/* Recalculate CRC of control file */
647656
INIT_CRC32C(ControlFile->crc);
@@ -651,16 +660,16 @@ updateControlFile(ControlFileData *ControlFile)
651660
FIN_CRC32C(ControlFile->crc);
652661

653662
/*
654-
* Write outPG_CONTROL_SIZE bytes into pg_control by zero-padding the
655-
* excess over sizeof(ControlFileData) to avoid premature EOF related
663+
* Write outPG_CONTROL_FILE_SIZE bytes into pg_control by zero-padding
664+
*theexcess over sizeof(ControlFileData), to avoid premature EOF related
656665
* errors when reading it.
657666
*/
658-
memset(buffer,0,PG_CONTROL_SIZE);
667+
memset(buffer,0,PG_CONTROL_FILE_SIZE);
659668
memcpy(buffer,ControlFile,sizeof(ControlFileData));
660669

661670
open_target_file("global/pg_control", false);
662671

663-
write_target_range(buffer,0,PG_CONTROL_SIZE);
672+
write_target_range(buffer,0,PG_CONTROL_FILE_SIZE);
664673

665674
close_target_file();
666675
}

‎src/include/catalog/pg_control.h

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,12 @@
2020
#include"port/pg_crc32c.h"
2121

2222

23-
#defineMOCK_AUTH_NONCE_LEN32
24-
2523
/* Version identifier for this pg_control format */
2624
#definePG_CONTROL_VERSION1002
2725

26+
/* Nonce key length, see below */
27+
#defineMOCK_AUTH_NONCE_LEN32
28+
2829
/*
2930
* Body of CheckPoint XLOG records. This is declared here because we keep
3031
* a copy of the latest one in pg_control for possible disaster recovery.
@@ -94,10 +95,6 @@ typedef enum DBState
9495

9596
/*
9697
* Contents of pg_control.
97-
*
98-
* NOTE: try to keep this under 512 bytes so that it will fit on one physical
99-
* sector of typical disk drives. This reduces the odds of corruption due to
100-
* power failure midway through a write.
10198
*/
10299

103100
typedefstructControlFileData
@@ -235,13 +232,21 @@ typedef struct ControlFileData
235232
pg_crc32ccrc;
236233
}ControlFileData;
237234

235+
/*
236+
* Maximum safe value of sizeof(ControlFileData). For reliability's sake,
237+
* it's critical that pg_control updates be atomic writes. That generally
238+
* means the active data can't be more than one disk sector, which is 512
239+
* bytes on common hardware. Be very careful about raising this limit.
240+
*/
241+
#definePG_CONTROL_MAX_SAFE_SIZE512
242+
238243
/*
239244
* Physical size of the pg_control file. Note that this is considerably
240245
* bigger than the actually used size (ie, sizeof(ControlFileData)).
241246
* The idea is to keep the physical size constant independent of format
242247
* changes, so that ReadControlFile will deliver a suitable wrong-version
243248
* message instead of a read error if it's looking at an incompatible file.
244249
*/
245-
#definePG_CONTROL_SIZE8192
250+
#definePG_CONTROL_FILE_SIZE8192
246251

247252
#endif/* PG_CONTROL_H */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp