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

Commit6606c57

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 parent7060f80 commit6606c57

File tree

3 files changed

+545
-39
lines changed

3 files changed

+545
-39
lines changed

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

Lines changed: 38 additions & 39 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-
(uint32) (RecPtr >>32), (uint32)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,17 +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,
431-
(uint32) (RecPtr >>32), (uint32)RecPtr);
432-
gotoerr;
433-
}
402+
Assert(state->readRecordBufSize >=XLOG_BLCKSZ*2);
403+
Assert(state->readRecordBufSize >=len);
434404

435405
/* Copy the first fragment of the record from the first page. */
436406
memcpy(state->readRecordBuf,
@@ -525,8 +495,37 @@ XLogReadRecord(XLogReaderState *state, char **errormsg)
525495
gotoerr;
526496
gotheader= true;
527497
}
528-
}while (gotlen<total_len);
529498

499+
/*
500+
* We might need a bigger buffer. We have validated the record
501+
* header, in the case that it split over a page boundary. We've
502+
* also cross-checked total_len against xlp_rem_len on the second
503+
* page, and verified xlp_pageaddr on both.
504+
*/
505+
Assert(gotheader);
506+
if (total_len>state->readRecordBufSize)
507+
{
508+
charsave_copy[XLOG_BLCKSZ*2];
509+
510+
/*
511+
* Save and restore the data we already had. It can't be more
512+
* than two pages.
513+
*/
514+
Assert(gotlen <=lengthof(save_copy));
515+
Assert(gotlen <=state->readRecordBufSize);
516+
memcpy(save_copy,state->readRecordBuf,gotlen);
517+
if (!allocate_recordbuf(state,total_len))
518+
{
519+
/* We treat this as a "bogus data" condition */
520+
report_invalid_record(state,"record length %u at %X/%X too long",
521+
total_len,
522+
(uint32) (RecPtr >>32), (uint32)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
@@ -66,6 +66,7 @@ our @EXPORT = qw(
6666
check_mode_recursive
6767
chmod_recursive
6868
check_pg_config
69+
scan_server_header
6970
system_or_bail
7071
system_log
7172
run_log
@@ -632,6 +633,46 @@ sub chmod_recursive
632633

633634
=pod
634635
636+
=itemscan_server_header(header_path, regexp)
637+
638+
Returns an array that stores all the matches of the given regular expression
639+
within the PostgreSQL installation'sC<header_path>. This can be used to
640+
retrieve specific value patterns from the installation's header files.
641+
642+
=cut
643+
644+
subscan_server_header
645+
{
646+
my ($header_path,$regexp) =@_;
647+
648+
my ($stdout,$stderr);
649+
my$result = IPC::Run::run ['pg_config','--includedir-server' ],'>',
650+
\$stdout,'2>', \$stderr
651+
ordie"could not execute pg_config";
652+
chomp($stdout);
653+
$stdout =~s/\r$//;
654+
655+
openmy$header_h,'<',"$stdout/$header_path"ordie"$!";
656+
657+
my@match =undef;
658+
while (<$header_h>)
659+
{
660+
my$line =$_;
661+
662+
if (@match =$line =~/^$regexp/)
663+
{
664+
last;
665+
}
666+
}
667+
668+
close$header_h;
669+
die"could not find match in header$header_path\n"
670+
unless@match;
671+
return@match;
672+
}
673+
674+
=pod
675+
635676
=itemcheck_pg_config(regexp)
636677
637678
Return the number of matches of the given regular expression

0 commit comments

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