From e79e6c4da152154544913b38354b98d07b65e411 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 28 Sep 2016 12:00:00 -0400 Subject: [PATCH] Fix CRC check handling in get_controlfile The previous patch broke this by returning NULL for a failed CRC check, which pg_controldata would then try to read. Fix by returning the result of the CRC check in a separate argument. Michael Paquier and myself --- src/backend/utils/misc/pg_controldata.c | 20 ++++++++++++-------- src/bin/pg_controldata/pg_controldata.c | 5 +++-- src/bin/pg_ctl/pg_ctl.c | 24 +++++++----------------- src/common/controldata_utils.c | 19 +++++++++---------- src/include/common/controldata_utils.h | 2 +- 5 files changed, 32 insertions(+), 38 deletions(-) diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c index 4f9de83097..0c67833f60 100644 --- a/src/backend/utils/misc/pg_controldata.c +++ b/src/backend/utils/misc/pg_controldata.c @@ -34,6 +34,7 @@ pg_control_system(PG_FUNCTION_ARGS) TupleDesc tupdesc; HeapTuple htup; ControlFileData *ControlFile; + bool crc_ok; /* * Construct a tuple descriptor for the result row. This must match this @@ -51,8 +52,8 @@ pg_control_system(PG_FUNCTION_ARGS) tupdesc = BlessTupleDesc(tupdesc); /* read the control file */ - ControlFile = get_controlfile(DataDir, NULL); - if (!ControlFile) + ControlFile = get_controlfile(DataDir, NULL, &crc_ok); + if (!crc_ok) ereport(ERROR, (errmsg("calculated CRC checksum does not match value stored in file"))); @@ -83,6 +84,7 @@ pg_control_checkpoint(PG_FUNCTION_ARGS) ControlFileData *ControlFile; XLogSegNo segno; char xlogfilename[MAXFNAMELEN]; + bool crc_ok; /* * Construct a tuple descriptor for the result row. This must match this @@ -130,8 +132,8 @@ pg_control_checkpoint(PG_FUNCTION_ARGS) tupdesc = BlessTupleDesc(tupdesc); /* Read the control file. */ - ControlFile = get_controlfile(DataDir, NULL); - if (!ControlFile) + ControlFile = get_controlfile(DataDir, NULL, &crc_ok); + if (!crc_ok) ereport(ERROR, (errmsg("calculated CRC checksum does not match value stored in file"))); @@ -216,6 +218,7 @@ pg_control_recovery(PG_FUNCTION_ARGS) TupleDesc tupdesc; HeapTuple htup; ControlFileData *ControlFile; + bool crc_ok; /* * Construct a tuple descriptor for the result row. This must match this @@ -235,8 +238,8 @@ pg_control_recovery(PG_FUNCTION_ARGS) tupdesc = BlessTupleDesc(tupdesc); /* read the control file */ - ControlFile = get_controlfile(DataDir, NULL); - if (!ControlFile) + ControlFile = get_controlfile(DataDir, NULL, &crc_ok); + if (!crc_ok) ereport(ERROR, (errmsg("calculated CRC checksum does not match value stored in file"))); @@ -268,6 +271,7 @@ pg_control_init(PG_FUNCTION_ARGS) TupleDesc tupdesc; HeapTuple htup; ControlFileData *ControlFile; + bool crc_ok; /* * Construct a tuple descriptor for the result row. This must match this @@ -303,8 +307,8 @@ pg_control_init(PG_FUNCTION_ARGS) tupdesc = BlessTupleDesc(tupdesc); /* read the control file */ - ControlFile = get_controlfile(DataDir, NULL); - if (!ControlFile) + ControlFile = get_controlfile(DataDir, NULL, &crc_ok); + if (!crc_ok) ereport(ERROR, (errmsg("calculated CRC checksum does not match value stored in file"))); diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c index e92feabade..20077a6639 100644 --- a/src/bin/pg_controldata/pg_controldata.c +++ b/src/bin/pg_controldata/pg_controldata.c @@ -86,6 +86,7 @@ int main(int argc, char *argv[]) { ControlFileData *ControlFile; + bool crc_ok; char *DataDir = NULL; time_t time_tmp; char pgctime_str[128]; @@ -155,8 +156,8 @@ main(int argc, char *argv[]) } /* get a copy of the control file */ - ControlFile = get_controlfile(DataDir, progname); - if (!ControlFile) + ControlFile = get_controlfile(DataDir, progname, &crc_ok); + if (!crc_ok) printf(_("WARNING: Calculated CRC checksum does not match value stored in file.\n" "Either the file is corrupt, or it has a different layout than this program\n" "is expecting. The results below are untrustworthy.\n\n")); diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index 052b02e0ff..ab10d2f25c 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -2147,28 +2147,18 @@ static DBState get_control_dbstate(void) { DBState ret; + bool crc_ok; + ControlFileData *control_file_data = get_controlfile(pg_data, progname, &crc_ok); - for (;;) + if (!crc_ok) { - ControlFileData *control_file_data = get_controlfile(pg_data, progname); - - if (control_file_data) - { - ret = control_file_data->state; - pfree(control_file_data); - return ret; - } - - if (wait_seconds > 0) - { - pg_usleep(1000000); /* 1 sec */ - wait_seconds--; - continue; - } - write_stderr(_("%s: control file appears to be corrupt\n"), progname); exit(1); } + + ret = control_file_data->state; + pfree(control_file_data); + return ret; } diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c index f218d2558c..cbdae052a7 100644 --- a/src/common/controldata_utils.c +++ b/src/common/controldata_utils.c @@ -29,21 +29,24 @@ #include "port/pg_crc32c.h" /* - * get_controlfile(char *DataDir, const char *progname) + * get_controlfile(char *DataDir, const char *progname, bool *crc_ok_p) * - * Get controlfile values. The caller is responsible - * for pfreeing the result. + * Get controlfile values. The result is returned as a palloc'd copy of the + * control file data. * - * Returns NULL if the CRC did not match. + * crc_ok_p can be used by the caller to see whether the CRC of the control + * file data is correct. */ ControlFileData * -get_controlfile(const char *DataDir, const char *progname) +get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p) { ControlFileData *ControlFile; int fd; char ControlFilePath[MAXPGPATH]; pg_crc32c crc; + AssertArg(crc_ok_p); + ControlFile = palloc(sizeof(ControlFileData)); snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir); @@ -83,11 +86,7 @@ get_controlfile(const char *DataDir, const char *progname) offsetof(ControlFileData, crc)); FIN_CRC32C(crc); - if (!EQ_CRC32C(crc, ControlFile->crc)) - { - pfree(ControlFile); - return NULL; - } + *crc_ok_p = EQ_CRC32C(crc, ControlFile->crc); /* Make sure the control file is valid byte order. */ if (ControlFile->pg_control_version % 65536 == 0 && diff --git a/src/include/common/controldata_utils.h b/src/include/common/controldata_utils.h index f834624e4e..a822cba192 100644 --- a/src/include/common/controldata_utils.h +++ b/src/include/common/controldata_utils.h @@ -12,6 +12,6 @@ #include "catalog/pg_control.h" -extern ControlFileData *get_controlfile(const char *DataDir, const char *progname); +extern ControlFileData *get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p); #endif /* COMMON_CONTROLDATA_UTILS_H */ -- 2.40.0