Skip to content

Commit a8beece

Browse 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 4c95ce0 commit a8beece

File tree

5 files changed

+116
-53
lines changed

5 files changed

+116
-53
lines changed

‎src/backend/replication/basebackup.c

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,18 @@ static int64 total_checksum_failures;
123123
/* Do not verify checksums. */
124124
static bool noverify_checksums = false;
125125

126+
/*
127+
* Definition of one element part of an exclusion list, used for paths part
128+
* of checksum validation or base backups. "name" is the name of the file
129+
* or path to check for exclusion. If "match_prefix" is true, any items
130+
* matching the name as prefix are excluded.
131+
*/
132+
struct exclude_list_item
133+
{
134+
const char *name;
135+
bool match_prefix;
136+
};
137+
126138
/*
127139
* The contents of these directories are removed or recreated during server
128140
* start so they are not included in backups. The directories themselves are
@@ -172,31 +184,34 @@ static const char *excludeDirContents[] =
172184
/*
173185
* List of files excluded from backups.
174186
*/
175-
static const char *excludeFiles[] =
187+
static const struct exclude_list_item excludeFiles[] =
176188
{
177189
/* Skip auto conf temporary file. */
178-
PG_AUTOCONF_FILENAME ".tmp",
190+
{PG_AUTOCONF_FILENAME ".tmp", false},
179191

180192
/* Skip current log file temporary file */
181-
LOG_METAINFO_DATAFILE_TMP,
193+
{LOG_METAINFO_DATAFILE_TMP, false},
182194

183-
/* Skip relation cache because it is rebuilt on startup */
184-
RELCACHE_INIT_FILENAME,
195+
/*
196+
* Skip relation cache because it is rebuilt on startup. This includes
197+
* temporary files.
198+
*/
199+
{RELCACHE_INIT_FILENAME, true},
185200

186201
/*
187202
* If there's a backup_label or tablespace_map file, it belongs to a
188203
* backup started by the user with pg_start_backup(). It is *not* correct
189204
* for this backup. Our backup_label/tablespace_map is injected into the
190205
* tar separately.
191206
*/
192-
BACKUP_LABEL_FILE,
193-
TABLESPACE_MAP,
207+
{BACKUP_LABEL_FILE, false},
208+
{TABLESPACE_MAP, false},
194209

195-
"postmaster.pid",
196-
"postmaster.opts",
210+
{"postmaster.pid", false},
211+
{"postmaster.opts", false},
197212

198213
/* end of list */
199-
NULL
214+
{NULL, false}
200215
};
201216

202217
/*
@@ -205,16 +220,15 @@ static const char *excludeFiles[] =
205220
* Note: this list should be kept in sync with what pg_checksums.c
206221
* includes.
207222
*/
208-
static const char *const noChecksumFiles[] = {
209-
"pg_control",
210-
"pg_filenode.map",
211-
"pg_internal.init",
212-
"PG_VERSION",
223+
static const struct exclude_list_item noChecksumFiles[] = {
224+
{"pg_control", false},
225+
{"pg_filenode.map", false},
226+
{"pg_internal.init", true},
227+
{"PG_VERSION", false},
213228
#ifdef EXEC_BACKEND
214-
"config_exec_params",
215-
"config_exec_params.new",
229+
{"config_exec_params", true},
216230
#endif
217-
NULL,
231+
{NULL, false}
218232
};
219233

220234

@@ -1107,9 +1121,13 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
11071121

11081122
/* Scan for files that should be excluded */
11091123
excludeFound = false;
1110-
for (excludeIdx = 0; excludeFiles[excludeIdx] != NULL; excludeIdx++)
1124+
for (excludeIdx = 0; excludeFiles[excludeIdx].name != NULL; excludeIdx++)
11111125
{
1112-
if (strcmp(de->d_name, excludeFiles[excludeIdx]) == 0)
1126+
int cmplen = strlen(excludeFiles[excludeIdx].name);
1127+
1128+
if (!excludeFiles[excludeIdx].match_prefix)
1129+
cmplen++;
1130+
if (strncmp(de->d_name, excludeFiles[excludeIdx].name, cmplen) == 0)
11131131
{
11141132
elog(DEBUG1, "file \"%s\" excluded from backup", de->d_name);
11151133
excludeFound = true;
@@ -1342,17 +1360,24 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
13421360
static bool
13431361
is_checksummed_file(const char *fullpath, const char *filename)
13441362
{
1345-
const char *const *f;
1346-
13471363
/* Check that the file is in a tablespace */
13481364
if (strncmp(fullpath, "./global/", 9) == 0 ||
13491365
strncmp(fullpath, "./base/", 7) == 0 ||
13501366
strncmp(fullpath, "/", 1) == 0)
13511367
{
1352-
/* Compare file against noChecksumFiles skiplist */
1353-
for (f = noChecksumFiles; *f; f++)
1354-
if (strcmp(*f, filename) == 0)
1368+
int excludeIdx;
1369+
1370+
/* Compare file against noChecksumFiles skip list */
1371+
for (excludeIdx = 0; noChecksumFiles[excludeIdx].name != NULL; excludeIdx++)
1372+
{
1373+
int cmplen = strlen(noChecksumFiles[excludeIdx].name);
1374+
1375+
if (!noChecksumFiles[excludeIdx].match_prefix)
1376+
cmplen++;
1377+
if (strncmp(filename, noChecksumFiles[excludeIdx].name,
1378+
cmplen) == 0)
13551379
return false;
1380+
}
13561381

13571382
return true;
13581383
}

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

Lines 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

Lines 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

Lines 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

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,18 @@ static int final_filemap_cmp(const void *a, const void *b);
3131
static void filemap_list_to_array(filemap_t *map);
3232
static bool check_file_excluded(const char *path, bool is_source);
3333

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

8193
/*
82-
* List of files excluded from filemap processing.
94+
* List of files excluded from filemap processing. Files are excluded
95+
* if their prefix match.
8396
*/
84-
static const char *excludeFiles[] =
97+
static const struct exclude_list_item excludeFiles[] =
8598
{
8699
/* Skip auto conf temporary file. */
87-
"postgresql.auto.conf.tmp", /* defined as PG_AUTOCONF_FILENAME */
100+
{"postgresql.auto.conf.tmp", false}, /* defined as PG_AUTOCONF_FILENAME */
88101

89102
/* Skip current log file temporary file */
90-
"current_logfiles.tmp", /* defined as LOG_METAINFO_DATAFILE_TMP */
103+
{"current_logfiles.tmp", false}, /* defined as
104+
* LOG_METAINFO_DATAFILE_TMP */
91105

92106
/* Skip relation cache because it is rebuilt on startup */
93-
"pg_internal.init", /* defined as RELCACHE_INIT_FILENAME */
107+
{"pg_internal.init", true}, /* defined as RELCACHE_INIT_FILENAME */
94108

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

103-
"postmaster.pid",
104-
"postmaster.opts",
117+
{"postmaster.pid", false},
118+
{"postmaster.opts", false},
105119

106120
/* end of list */
107-
NULL
121+
{NULL, false}
108122
};
109123

110124
/*
@@ -497,14 +511,19 @@ check_file_excluded(const char *path, bool is_source)
497511
const char *filename;
498512

499513
/* check individual files... */
500-
for (excludeIdx = 0; excludeFiles[excludeIdx] != NULL; excludeIdx++)
514+
for (excludeIdx = 0; excludeFiles[excludeIdx].name != NULL; excludeIdx++)
501515
{
516+
int cmplen = strlen(excludeFiles[excludeIdx].name);
517+
502518
filename = last_dir_separator(path);
503519
if (filename == NULL)
504520
filename = path;
505521
else
506522
filename++;
507-
if (strcmp(filename, excludeFiles[excludeIdx]) == 0)
523+
524+
if (!excludeFiles[excludeIdx].match_prefix)
525+
cmplen++;
526+
if (strncmp(filename, excludeFiles[excludeIdx].name, cmplen) == 0)
508527
{
509528
if (is_source)
510529
pg_log_debug("entry \"%s\" excluded from source file list",

0 commit comments

Comments
 (0)