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
Appearance settings

Commit bf883b2

Browse filesBrowse files
michaelpqmichaelpq
committed
Add prefix checks in exclude lists for pg_rewind, pg_checksums and base backups
An instance of PostgreSQL crashing with a bad timing could leave behind temporary pg_internal.init files, potentially causing failures when verifying checksums. As the same exclusion lists are used between pg_rewind, pg_checksums and basebackup.c, all those tools are extended with prefix checks to keep everything in sync, with dedicated checks added for pg_internal.init. Backpatch down to 11, where pg_checksums (pg_verify_checksums in 11) and checksum verification for base backups have been introduced. Reported-by: Michael Banck Author: Michael Paquier Reviewed-by: Kyotaro Horiguchi, David Steele Discussion: https://postgr.es/m/62031974fd8e941dd8351fbc8c7eff60d59c5338.camel@credativ.de Backpatch-through: 11
1 parent 79c2385 commit bf883b2
Copy full SHA for bf883b2

File tree

Expand file treeCollapse file tree

5 files changed

+116
-53
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+116
-53
lines changed

‎src/backend/replication/basebackup.c

Copy file name to clipboardExpand all lines: src/backend/replication/basebackup.c
+50-25Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,18 @@ static long long int total_checksum_failures;
121121
/* Do not verify checksums. */
122122
static bool noverify_checksums = false;
123123

124+
/*
125+
* Definition of one element part of an exclusion list, used for paths part
126+
* of checksum validation or base backups. "name" is the name of the file
127+
* or path to check for exclusion. If "match_prefix" is true, any items
128+
* matching the name as prefix are excluded.
129+
*/
130+
struct exclude_list_item
131+
{
132+
const char *name;
133+
bool match_prefix;
134+
};
135+
124136
/*
125137
* The contents of these directories are removed or recreated during server
126138
* start so they are not included in backups. The directories themselves are
@@ -170,31 +182,34 @@ static const char *const excludeDirContents[] =
170182
/*
171183
* List of files excluded from backups.
172184
*/
173-
static const char *const excludeFiles[] =
185+
static const struct exclude_list_item excludeFiles[] =
174186
{
175187
/* Skip auto conf temporary file. */
176-
PG_AUTOCONF_FILENAME ".tmp",
188+
{PG_AUTOCONF_FILENAME ".tmp", false},
177189

178190
/* Skip current log file temporary file */
179-
LOG_METAINFO_DATAFILE_TMP,
191+
{LOG_METAINFO_DATAFILE_TMP, false},
180192

181-
/* Skip relation cache because it is rebuilt on startup */
182-
RELCACHE_INIT_FILENAME,
193+
/*
194+
* Skip relation cache because it is rebuilt on startup. This includes
195+
* temporary files.
196+
*/
197+
{RELCACHE_INIT_FILENAME, true},
183198

184199
/*
185200
* If there's a backup_label or tablespace_map file, it belongs to a
186201
* backup started by the user with pg_start_backup(). It is *not* correct
187202
* for this backup. Our backup_label/tablespace_map is injected into the
188203
* tar separately.
189204
*/
190-
BACKUP_LABEL_FILE,
191-
TABLESPACE_MAP,
205+
{BACKUP_LABEL_FILE, false},
206+
{TABLESPACE_MAP, false},
192207

193-
"postmaster.pid",
194-
"postmaster.opts",
208+
{"postmaster.pid", false},
209+
{"postmaster.opts", false},
195210

196211
/* end of list */
197-
NULL
212+
{NULL, false}
198213
};
199214

200215
/*
@@ -203,16 +218,15 @@ static const char *const excludeFiles[] =
203218
* Note: this list should be kept in sync with what pg_checksums.c
204219
* includes.
205220
*/
206-
static const char *const noChecksumFiles[] = {
207-
"pg_control",
208-
"pg_filenode.map",
209-
"pg_internal.init",
210-
"PG_VERSION",
221+
static const struct exclude_list_item noChecksumFiles[] = {
222+
{"pg_control", false},
223+
{"pg_filenode.map", false},
224+
{"pg_internal.init", true},
225+
{"PG_VERSION", false},
211226
#ifdef EXEC_BACKEND
212-
"config_exec_params",
213-
"config_exec_params.new",
227+
{"config_exec_params", true},
214228
#endif
215-
NULL,
229+
{NULL, false}
216230
};
217231

218232
/*
@@ -1082,9 +1096,13 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
10821096

10831097
/* Scan for files that should be excluded */
10841098
excludeFound = false;
1085-
for (excludeIdx = 0; excludeFiles[excludeIdx] != NULL; excludeIdx++)
1099+
for (excludeIdx = 0; excludeFiles[excludeIdx].name != NULL; excludeIdx++)
10861100
{
1087-
if (strcmp(de->d_name, excludeFiles[excludeIdx]) == 0)
1101+
int cmplen = strlen(excludeFiles[excludeIdx].name);
1102+
1103+
if (!excludeFiles[excludeIdx].match_prefix)
1104+
cmplen++;
1105+
if (strncmp(de->d_name, excludeFiles[excludeIdx].name, cmplen) == 0)
10881106
{
10891107
elog(DEBUG1, "file \"%s\" excluded from backup", de->d_name);
10901108
excludeFound = true;
@@ -1317,17 +1335,24 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
13171335
static bool
13181336
is_checksummed_file(const char *fullpath, const char *filename)
13191337
{
1320-
const char *const *f;
1321-
13221338
/* Check that the file is in a tablespace */
13231339
if (strncmp(fullpath, "./global/", 9) == 0 ||
13241340
strncmp(fullpath, "./base/", 7) == 0 ||
13251341
strncmp(fullpath, "/", 1) == 0)
13261342
{
1327-
/* Compare file against noChecksumFiles skiplist */
1328-
for (f = noChecksumFiles; *f; f++)
1329-
if (strcmp(*f, filename) == 0)
1343+
int excludeIdx;
1344+
1345+
/* Compare file against noChecksumFiles skip list */
1346+
for (excludeIdx = 0; noChecksumFiles[excludeIdx].name != NULL; excludeIdx++)
1347+
{
1348+
int cmplen = strlen(noChecksumFiles[excludeIdx].name);
1349+
1350+
if (!noChecksumFiles[excludeIdx].match_prefix)
1351+
cmplen++;
1352+
if (strncmp(filename, noChecksumFiles[excludeIdx].name,
1353+
cmplen) == 0)
13301354
return false;
1355+
}
13311356

13321357
return true;
13331358
}

‎src/bin/pg_basebackup/t/010_pg_basebackup.pl

Copy file name to clipboardExpand all lines: src/bin/pg_basebackup/t/010_pg_basebackup.pl
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use File::Path qw(rmtree);
77
use PostgresNode;
88
use TestLib;
9-
use Test::More tests => 106;
9+
use Test::More tests => 107;
1010

1111
program_help_ok('pg_basebackup');
1212
program_version_ok('pg_basebackup');
@@ -65,8 +65,8 @@
6565

6666
# Write some files to test that they are not copied.
6767
foreach my $filename (
68-
qw(backup_label tablespace_map postgresql.auto.conf.tmp current_logfiles.tmp)
69-
)
68+
qw(backup_label tablespace_map postgresql.auto.conf.tmp
69+
current_logfiles.tmp global/pg_internal.init.123))
7070
{
7171
open my $file, '>>', "$pgdata/$filename";
7272
print $file "DONOTCOPY";
@@ -135,7 +135,7 @@
135135
# These files should not be copied.
136136
foreach my $filename (
137137
qw(postgresql.auto.conf.tmp postmaster.opts postmaster.pid tablespace_map current_logfiles.tmp
138-
global/pg_internal.init))
138+
global/pg_internal.init global/pg_internal.init.123))
139139
{
140140
ok(!-f "$tempdir/backup/$filename", "$filename not copied");
141141
}

‎src/bin/pg_checksums/pg_checksums.c

Copy file name to clipboardExpand all lines: src/bin/pg_checksums/pg_checksums.c
+28-11Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,21 +91,32 @@ usage(void)
9191
printf(_("Report bugs to <pgsql-bugs@lists.postgresql.org>.\n"));
9292
}
9393

94+
/*
95+
* Definition of one element part of an exclusion list, used for files
96+
* to exclude from checksum validation. "name" is the name of the file
97+
* or path to check for exclusion. If "match_prefix" is true, any items
98+
* matching the name as prefix are excluded.
99+
*/
100+
struct exclude_list_item
101+
{
102+
const char *name;
103+
bool match_prefix;
104+
};
105+
94106
/*
95107
* List of files excluded from checksum validation.
96108
*
97109
* Note: this list should be kept in sync with what basebackup.c includes.
98110
*/
99-
static const char *const skip[] = {
100-
"pg_control",
101-
"pg_filenode.map",
102-
"pg_internal.init",
103-
"PG_VERSION",
111+
static const struct exclude_list_item skip[] = {
112+
{"pg_control", false},
113+
{"pg_filenode.map", false},
114+
{"pg_internal.init", true},
115+
{"PG_VERSION", false},
104116
#ifdef EXEC_BACKEND
105-
"config_exec_params",
106-
"config_exec_params.new",
117+
{"config_exec_params", true},
107118
#endif
108-
NULL,
119+
{NULL, false}
109120
};
110121

111122
/*
@@ -157,11 +168,17 @@ progress_report(bool force)
157168
static bool
158169
skipfile(const char *fn)
159170
{
160-
const char *const *f;
171+
int excludeIdx;
172+
173+
for (excludeIdx = 0; skip[excludeIdx].name != NULL; excludeIdx++)
174+
{
175+
int cmplen = strlen(skip[excludeIdx].name);
161176

162-
for (f = skip; *f; f++)
163-
if (strcmp(*f, fn) == 0)
177+
if (!skip[excludeIdx].match_prefix)
178+
cmplen++;
179+
if (strncmp(skip[excludeIdx].name, fn, cmplen) == 0)
164180
return true;
181+
}
165182

166183
return false;
167184
}

‎src/bin/pg_checksums/t/002_actions.pl

Copy file name to clipboardExpand all lines: src/bin/pg_checksums/t/002_actions.pl
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,9 @@ sub check_relation_corruption
111111
# should be ignored by the scan.
112112
append_to_file "$pgdata/global/pgsql_tmp_123", "foo";
113113
mkdir "$pgdata/global/pgsql_tmp";
114-
append_to_file "$pgdata/global/pgsql_tmp/1.1", "foo";
114+
append_to_file "$pgdata/global/pgsql_tmp/1.1", "foo";
115+
append_to_file "$pgdata/global/pg_internal.init", "foo";
116+
append_to_file "$pgdata/global/pg_internal.init.123", "foo";
115117

116118
# Enable checksums.
117119
command_ok([ 'pg_checksums', '--enable', '--no-sync', '-D', $pgdata ],

‎src/bin/pg_rewind/filemap.c

Copy file name to clipboardExpand all lines: src/bin/pg_rewind/filemap.c
+31-12Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,18 @@ static int final_filemap_cmp(const void *a, const void *b);
3030
static void filemap_list_to_array(filemap_t *map);
3131
static bool check_file_excluded(const char *path, bool is_source);
3232

33+
/*
34+
* Definition of one element part of an exclusion list, used to exclude
35+
* contents when rewinding. "name" is the name of the file or path to
36+
* check for exclusion. If "match_prefix" is true, any items matching
37+
* the name as prefix are excluded.
38+
*/
39+
struct exclude_list_item
40+
{
41+
const char *name;
42+
bool match_prefix;
43+
};
44+
3345
/*
3446
* The contents of these directories are removed or recreated during server
3547
* start so they are not included in data processed by pg_rewind.
@@ -78,32 +90,34 @@ static const char *excludeDirContents[] =
7890
};
7991

8092
/*
81-
* List of files excluded from filemap processing.
93+
* List of files excluded from filemap processing. Files are excluded
94+
* if their prefix match.
8295
*/
83-
static const char *excludeFiles[] =
96+
static const struct exclude_list_item excludeFiles[] =
8497
{
8598
/* Skip auto conf temporary file. */
86-
"postgresql.auto.conf.tmp", /* defined as PG_AUTOCONF_FILENAME */
99+
{"postgresql.auto.conf.tmp", false}, /* defined as PG_AUTOCONF_FILENAME */
87100

88101
/* Skip current log file temporary file */
89-
"current_logfiles.tmp", /* defined as LOG_METAINFO_DATAFILE_TMP */
102+
{"current_logfiles.tmp", false}, /* defined as
103+
* LOG_METAINFO_DATAFILE_TMP */
90104

91105
/* Skip relation cache because it is rebuilt on startup */
92-
"pg_internal.init", /* defined as RELCACHE_INIT_FILENAME */
106+
{"pg_internal.init", true}, /* defined as RELCACHE_INIT_FILENAME */
93107

94108
/*
95109
* If there's a backup_label or tablespace_map file, it belongs to a
96110
* backup started by the user with pg_start_backup(). It is *not* correct
97111
* for this backup. Our backup_label is written later on separately.
98112
*/
99-
"backup_label", /* defined as BACKUP_LABEL_FILE */
100-
"tablespace_map", /* defined as TABLESPACE_MAP */
113+
{"backup_label", false}, /* defined as BACKUP_LABEL_FILE */
114+
{"tablespace_map", false}, /* defined as TABLESPACE_MAP */
101115

102-
"postmaster.pid",
103-
"postmaster.opts",
116+
{"postmaster.pid", false},
117+
{"postmaster.opts", false},
104118

105119
/* end of list */
106-
NULL
120+
{NULL, false}
107121
};
108122

109123
/*
@@ -496,14 +510,19 @@ check_file_excluded(const char *path, bool is_source)
496510
const char *filename;
497511

498512
/* check individual files... */
499-
for (excludeIdx = 0; excludeFiles[excludeIdx] != NULL; excludeIdx++)
513+
for (excludeIdx = 0; excludeFiles[excludeIdx].name != NULL; excludeIdx++)
500514
{
515+
int cmplen = strlen(excludeFiles[excludeIdx].name);
516+
501517
filename = last_dir_separator(path);
502518
if (filename == NULL)
503519
filename = path;
504520
else
505521
filename++;
506-
if (strcmp(filename, excludeFiles[excludeIdx]) == 0)
522+
523+
if (!excludeFiles[excludeIdx].match_prefix)
524+
cmplen++;
525+
if (strncmp(filename, excludeFiles[excludeIdx].name, cmplen) == 0)
507526
{
508527
if (is_source)
509528
pg_log_debug("entry \"%s\" excluded from source file list",

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.