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

Commit3ce3b53

Browse files
committed
Don't trust unvalidated xl_tot_len.
xl_tot_len comes first in a WAL record. Usually we don't trust it to bethe true length until we've validated the record header. If the recordheader was split across two pages, previously we wouldn't do thevalidation until after we'd already tried to allocate enough memory tohold the record, which was bad because it might actually be garbagebytes from a recycled WAL file, so we could try to allocate a lot ofmemory. Release 15 made it worse.Since70b4f82, we'd at least generate an end-of-WAL condition if thegarbage 4 byte value happened to be > 1GB, but we'd still try toallocate up to 1GB of memory bogusly otherwise. That was animprovement, but unfortunately release 15 tries to allocate anotherobject before that, so you could get a FATAL error and recovery couldfail.We can fix both variants of the problem more fundamentally usingpre-existing page-level validation, if we just re-order some logic.The new order of operations in the split-header case defers all memoryallocation based on xl_tot_len until we've read the following page. Atthat point we know that its first few bytes are not recycled data, bychecking its xlp_pageaddr, and that its xlp_rem_len agrees withxl_tot_len on the preceding page. That is strong evidence thatxl_tot_len was truly the start of a record that was logged.This problem was most likely to occur on a standby, becausewalreceiver.c recycles WAL files without zeroing out trailing regions ofeach page. We could fix that too, but it wouldn't protect us from rarecrash scenarios where the trailing zeroes don't make it to disk.With reliable xl_tot_len validation in place, the ancient policy ofconsidering malloc failure to indicate corruption at end-of-WAL seemsquite surprising, but changing that is left for later work.Also included is a new TAP test to exercise various cases of end-of-WALdetection by writing contrived data into the WAL from Perl.Back-patch to 12. We decided not to put this change into the finalrelease of 11.Author: Thomas Munro <thomas.munro@gmail.com>Author: Michael Paquier <michael@paquier.xyz>Reported-by: Alexander Lakhin <exclusion@gmail.com>Reviewed-by: Noah Misch <noah@leadboat.com> (the idea, not the code)Reviewed-by: Michael Paquier <michael@paquier.xyz>Reviewed-by: Sergei Kornilov <sk@zsrv.org>Reviewed-by: Alexander Lakhin <exclusion@gmail.com>Discussion:https://postgr.es/m/17928-aa92416a70ff44a2%40postgresql.org
1 parentbbb9bec commit3ce3b53

File tree

3 files changed

+544
-38
lines changed

3 files changed

+544
-38
lines changed

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

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@ XLogReaderFree(XLogReaderState *state)
168168
* XLOG_BLCKSZ, and make sure it's at least 5*Max(BLCKSZ, XLOG_BLCKSZ) to start
169169
* with. (That is enough for all "normal" records, but very large commit or
170170
* abort records might need more space.)
171+
*
172+
* Note: This routine should *never* be called for xl_tot_len until the header
173+
* of the record has been fully validated.
171174
*/
172175
staticbool
173176
allocate_recordbuf(XLogReaderState*state,uint32reclength)
@@ -177,25 +180,6 @@ allocate_recordbuf(XLogReaderState *state, uint32 reclength)
177180
newSize+=XLOG_BLCKSZ- (newSize %XLOG_BLCKSZ);
178181
newSize=Max(newSize,5*Max(BLCKSZ,XLOG_BLCKSZ));
179182

180-
#ifndefFRONTEND
181-
182-
/*
183-
* Note that in much unlucky circumstances, the random data read from a
184-
* recycled segment can cause this routine to be called with a size
185-
* causing a hard failure at allocation. For a standby, this would cause
186-
* the instance to stop suddenly with a hard failure, preventing it to
187-
* retry fetching WAL from one of its sources which could allow it to move
188-
* on with replay without a manual restart. If the data comes from a past
189-
* recycled segment and is still valid, then the allocation may succeed
190-
* but record checks are going to fail so this would be short-lived. If
191-
* the allocation fails because of a memory shortage, then this is not a
192-
* hard failure either per the guarantee given by MCXT_ALLOC_NO_OOM.
193-
*/
194-
if (!AllocSizeIsValid(newSize))
195-
return false;
196-
197-
#endif
198-
199183
if (state->readRecordBuf)
200184
pfree(state->readRecordBuf);
201185
state->readRecordBuf=
@@ -396,15 +380,7 @@ XLogReadRecord(XLogReaderState *state, char **errormsg)
396380
}
397381
else
398382
{
399-
/* XXX: more validation should be done here */
400-
if (total_len<SizeOfXLogRecord)
401-
{
402-
report_invalid_record(state,
403-
"invalid record length at %X/%X: wanted %u, got %u",
404-
LSN_FORMAT_ARGS(RecPtr),
405-
(uint32)SizeOfXLogRecord,total_len);
406-
gotoerr;
407-
}
383+
/* We'll validate the header once we have the next page. */
408384
gotheader= false;
409385
}
410386

@@ -420,16 +396,11 @@ XLogReadRecord(XLogReaderState *state, char **errormsg)
420396
assembled= true;
421397

422398
/*
423-
* Enlarge readRecordBuf as needed.
399+
* We always have space for a couple of pages, enough to validate a
400+
* boundary-spanning record header.
424401
*/
425-
if (total_len>state->readRecordBufSize&&
426-
!allocate_recordbuf(state,total_len))
427-
{
428-
/* We treat this as a "bogus data" condition */
429-
report_invalid_record(state,"record length %u at %X/%X too long",
430-
total_len,LSN_FORMAT_ARGS(RecPtr));
431-
gotoerr;
432-
}
402+
Assert(state->readRecordBufSize >=XLOG_BLCKSZ*2);
403+
Assert(state->readRecordBufSize >=len);
433404

434405
/* Copy the first fragment of the record from the first page. */
435406
memcpy(state->readRecordBuf,
@@ -525,8 +496,36 @@ XLogReadRecord(XLogReaderState *state, char **errormsg)
525496
gotoerr;
526497
gotheader= true;
527498
}
528-
}while (gotlen<total_len);
529499

500+
/*
501+
* We might need a bigger buffer. We have validated the record
502+
* header, in the case that it split over a page boundary. We've
503+
* also cross-checked total_len against xlp_rem_len on the second
504+
* page, and verified xlp_pageaddr on both.
505+
*/
506+
Assert(gotheader);
507+
if (total_len>state->readRecordBufSize)
508+
{
509+
charsave_copy[XLOG_BLCKSZ*2];
510+
511+
/*
512+
* Save and restore the data we already had. It can't be more
513+
* than two pages.
514+
*/
515+
Assert(gotlen <=lengthof(save_copy));
516+
Assert(gotlen <=state->readRecordBufSize);
517+
memcpy(save_copy,state->readRecordBuf,gotlen);
518+
if (!allocate_recordbuf(state,total_len))
519+
{
520+
/* We treat this as a "bogus data" condition */
521+
report_invalid_record(state,"record length %u at %X/%X too long",
522+
total_len,LSN_FORMAT_ARGS(RecPtr));
523+
gotoerr;
524+
}
525+
memcpy(state->readRecordBuf,save_copy,gotlen);
526+
buffer=state->readRecordBuf+gotlen;
527+
}
528+
}while (gotlen<total_len);
530529
Assert(gotheader);
531530

532531
record= (XLogRecord*)state->readRecordBuf;

‎src/test/perl/TestLib.pm

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ our @EXPORT = qw(
7070
chmod_recursive
7171
check_pg_config
7272
dir_symlink
73+
scan_server_header
7374
system_or_bail
7475
system_log
7576
run_log
@@ -648,6 +649,46 @@ sub chmod_recursive
648649

649650
=pod
650651
652+
=itemscan_server_header(header_path, regexp)
653+
654+
Returns an array that stores all the matches of the given regular expression
655+
within the PostgreSQL installation'sC<header_path>. This can be used to
656+
retrieve specific value patterns from the installation's header files.
657+
658+
=cut
659+
660+
subscan_server_header
661+
{
662+
my ($header_path,$regexp) =@_;
663+
664+
my ($stdout,$stderr);
665+
my$result = IPC::Run::run ['pg_config','--includedir-server' ],'>',
666+
\$stdout,'2>', \$stderr
667+
ordie"could not execute pg_config";
668+
chomp($stdout);
669+
$stdout =~s/\r$//;
670+
671+
openmy$header_h,'<',"$stdout/$header_path"ordie"$!";
672+
673+
my@match =undef;
674+
while (<$header_h>)
675+
{
676+
my$line =$_;
677+
678+
if (@match =$line =~/^$regexp/)
679+
{
680+
last;
681+
}
682+
}
683+
684+
close$header_h;
685+
die"could not find match in header$header_path\n"
686+
unless@match;
687+
return@match;
688+
}
689+
690+
=pod
691+
651692
=itemcheck_pg_config(regexp)
652693
653694
Return the number of matches of the given regular expression

0 commit comments

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