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

Commitf4d152e

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 that wouldn't protect us fromrare crash 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 parentbd82b8d commitf4d152e

File tree

3 files changed

+568
-56
lines changed

3 files changed

+568
-56
lines changed

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

Lines changed: 67 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,9 @@ XLogReaderFree(XLogReaderState *state)
192192
* XLOG_BLCKSZ, and make sure it's at least 5*Max(BLCKSZ, XLOG_BLCKSZ) to start
193193
* with. (That is enough for all "normal" records, but very large commit or
194194
* abort records might need more space.)
195+
*
196+
* Note: This routine should *never* be called for xl_tot_len until the header
197+
* of the record has been fully validated.
195198
*/
196199
staticbool
197200
allocate_recordbuf(XLogReaderState*state,uint32reclength)
@@ -201,25 +204,6 @@ allocate_recordbuf(XLogReaderState *state, uint32 reclength)
201204
newSize+=XLOG_BLCKSZ- (newSize %XLOG_BLCKSZ);
202205
newSize=Max(newSize,5*Max(BLCKSZ,XLOG_BLCKSZ));
203206

204-
#ifndefFRONTEND
205-
206-
/*
207-
* Note that in much unlucky circumstances, the random data read from a
208-
* recycled segment can cause this routine to be called with a size
209-
* causing a hard failure at allocation. For a standby, this would cause
210-
* the instance to stop suddenly with a hard failure, preventing it to
211-
* retry fetching WAL from one of its sources which could allow it to move
212-
* on with replay without a manual restart. If the data comes from a past
213-
* recycled segment and is still valid, then the allocation may succeed
214-
* but record checks are going to fail so this would be short-lived. If
215-
* the allocation fails because of a memory shortage, then this is not a
216-
* hard failure either per the guarantee given by MCXT_ALLOC_NO_OOM.
217-
*/
218-
if (!AllocSizeIsValid(newSize))
219-
return false;
220-
221-
#endif
222-
223207
if (state->readRecordBuf)
224208
pfree(state->readRecordBuf);
225209
state->readRecordBuf=
@@ -667,41 +651,26 @@ XLogDecodeNextRecord(XLogReaderState *state, bool nonblocking)
667651
}
668652
else
669653
{
670-
/* XXX: more validation should be done here */
671-
if (total_len<SizeOfXLogRecord)
672-
{
673-
report_invalid_record(state,
674-
"invalid record length at %X/%X: wanted %u, got %u",
675-
LSN_FORMAT_ARGS(RecPtr),
676-
(uint32)SizeOfXLogRecord,total_len);
677-
gotoerr;
678-
}
654+
/* We'll validate the header once we have the next page. */
679655
gotheader= false;
680656
}
681657

682658
/*
683-
* Find space to decode this record. Don't allow oversized allocation if
684-
* the caller requested nonblocking. Otherwise, we *have* to try to
685-
* decode the record now because the caller has nothing else to do, so
686-
* allow an oversized record to be palloc'd if that turns out to be
687-
* necessary.
659+
* Try to find space to decode this record, if we can do so without
660+
* calling palloc. If we can't, we'll try again below after we've
661+
* validated that total_len isn't garbage bytes from a recycled WAL page.
688662
*/
689663
decoded=XLogReadRecordAlloc(state,
690664
total_len,
691-
!nonblocking/* allow_oversized */ );
692-
if (decoded==NULL)
665+
false/* allow_oversized */ );
666+
if (decoded==NULL&&nonblocking)
693667
{
694668
/*
695-
* There is no space in the decode buffer. The caller should help
696-
* with that problem by consuming some records.
669+
* There is no space in the circular decode buffer, and the caller is
670+
* only reading ahead. The caller should consume existing records to
671+
* make space.
697672
*/
698-
if (nonblocking)
699-
returnXLREAD_WOULDBLOCK;
700-
701-
/* We failed to allocate memory for an oversized record. */
702-
report_invalid_record(state,
703-
"out of memory while trying to decode a record of length %u",total_len);
704-
gotoerr;
673+
returnXLREAD_WOULDBLOCK;
705674
}
706675

707676
len=XLOG_BLCKSZ-RecPtr %XLOG_BLCKSZ;
@@ -716,16 +685,11 @@ XLogDecodeNextRecord(XLogReaderState *state, bool nonblocking)
716685
assembled= true;
717686

718687
/*
719-
* Enlarge readRecordBuf as needed.
688+
* We always have space for a couple of pages, enough to validate a
689+
* boundary-spanning record header.
720690
*/
721-
if (total_len>state->readRecordBufSize&&
722-
!allocate_recordbuf(state,total_len))
723-
{
724-
/* We treat this as a "bogus data" condition */
725-
report_invalid_record(state,"record length %u at %X/%X too long",
726-
total_len,LSN_FORMAT_ARGS(RecPtr));
727-
gotoerr;
728-
}
691+
Assert(state->readRecordBufSize >=XLOG_BLCKSZ*2);
692+
Assert(state->readRecordBufSize >=len);
729693

730694
/* Copy the first fragment of the record from the first page. */
731695
memcpy(state->readRecordBuf,
@@ -822,8 +786,35 @@ XLogDecodeNextRecord(XLogReaderState *state, bool nonblocking)
822786
gotoerr;
823787
gotheader= true;
824788
}
825-
}while (gotlen<total_len);
826789

790+
/*
791+
* We might need a bigger buffer. We have validated the record
792+
* header, in the case that it split over a page boundary. We've
793+
* also cross-checked total_len against xlp_rem_len on the second
794+
* page, and verified xlp_pageaddr on both.
795+
*/
796+
if (total_len>state->readRecordBufSize)
797+
{
798+
charsave_copy[XLOG_BLCKSZ*2];
799+
800+
/*
801+
* Save and restore the data we already had. It can't be more
802+
* than two pages.
803+
*/
804+
Assert(gotlen <=lengthof(save_copy));
805+
Assert(gotlen <=state->readRecordBufSize);
806+
memcpy(save_copy,state->readRecordBuf,gotlen);
807+
if (!allocate_recordbuf(state,total_len))
808+
{
809+
/* We treat this as a "bogus data" condition */
810+
report_invalid_record(state,"record length %u at %X/%X too long",
811+
total_len,LSN_FORMAT_ARGS(RecPtr));
812+
gotoerr;
813+
}
814+
memcpy(state->readRecordBuf,save_copy,gotlen);
815+
buffer=state->readRecordBuf+gotlen;
816+
}
817+
}while (gotlen<total_len);
827818
Assert(gotheader);
828819

829820
record= (XLogRecord*)state->readRecordBuf;
@@ -865,6 +856,28 @@ XLogDecodeNextRecord(XLogReaderState *state, bool nonblocking)
865856
state->NextRecPtr-=XLogSegmentOffset(state->NextRecPtr,state->segcxt.ws_segsize);
866857
}
867858

859+
/*
860+
* If we got here without a DecodedXLogRecord, it means we needed to
861+
* validate total_len before trusting it, but by now now we've done that.
862+
*/
863+
if (decoded==NULL)
864+
{
865+
Assert(!nonblocking);
866+
decoded=XLogReadRecordAlloc(state,
867+
total_len,
868+
true/* allow_oversized */ );
869+
if (decoded==NULL)
870+
{
871+
/*
872+
* We failed to allocate memory for an oversized record. As
873+
* above, we currently treat this as a "bogus data" condition.
874+
*/
875+
report_invalid_record(state,
876+
"out of memory while trying to decode a record of length %u",total_len);
877+
gotoerr;
878+
}
879+
}
880+
868881
if (DecodeXLogRecord(state,decoded,record,RecPtr,&errormsg))
869882
{
870883
/* Record the location of the next record. */
@@ -893,8 +906,6 @@ XLogDecodeNextRecord(XLogReaderState *state, bool nonblocking)
893906
state->decode_queue_head=decoded;
894907
returnXLREAD_SUCCESS;
895908
}
896-
else
897-
returnXLREAD_FAIL;
898909

899910
err:
900911
if (assembled)

‎src/test/perl/PostgreSQL/Test/Utils.pm

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ our @EXPORT = qw(
6969
chmod_recursive
7070
check_pg_config
7171
dir_symlink
72+
scan_server_header
7273
system_or_bail
7374
system_log
7475
run_log
@@ -672,6 +673,46 @@ sub chmod_recursive
672673

673674
=pod
674675
676+
=itemscan_server_header(header_path, regexp)
677+
678+
Returns an array that stores all the matches of the given regular expression
679+
within the PostgreSQL installation'sC<header_path>. This can be used to
680+
retrieve specific value patterns from the installation's header files.
681+
682+
=cut
683+
684+
subscan_server_header
685+
{
686+
my ($header_path,$regexp) =@_;
687+
688+
my ($stdout,$stderr);
689+
my$result = IPC::Run::run ['pg_config','--includedir-server' ],'>',
690+
\$stdout,'2>', \$stderr
691+
ordie"could not execute pg_config";
692+
chomp($stdout);
693+
$stdout =~s/\r$//;
694+
695+
openmy$header_h,'<',"$stdout/$header_path"ordie"$!";
696+
697+
my@match =undef;
698+
while (<$header_h>)
699+
{
700+
my$line =$_;
701+
702+
if (@match =$line =~/^$regexp/)
703+
{
704+
last;
705+
}
706+
}
707+
708+
close$header_h;
709+
die"could not find match in header$header_path\n"
710+
unless@match;
711+
return@match;
712+
}
713+
714+
=pod
715+
675716
=itemcheck_pg_config(regexp)
676717
677718
Return the number of matches of the given regular expression

0 commit comments

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