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

Commite8f3c06

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 parentd7cc718 commite8f3c06

File tree

3 files changed

+538
-39
lines changed

3 files changed

+538
-39
lines changed

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

Lines changed: 38 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,9 @@ XLogReaderFree(XLogReaderState *state)
155155
* XLOG_BLCKSZ, and make sure it's at least 5*Max(BLCKSZ, XLOG_BLCKSZ) to start
156156
* with. (That is enough for all "normal" records, but very large commit or
157157
* abort records might need more space.)
158+
*
159+
* Note: This routine should *never* be called for xl_tot_len until the header
160+
* of the record has been fully validated.
158161
*/
159162
staticbool
160163
allocate_recordbuf(XLogReaderState*state,uint32reclength)
@@ -164,25 +167,6 @@ allocate_recordbuf(XLogReaderState *state, uint32 reclength)
164167
newSize+=XLOG_BLCKSZ- (newSize %XLOG_BLCKSZ);
165168
newSize=Max(newSize,5*Max(BLCKSZ,XLOG_BLCKSZ));
166169

167-
#ifndefFRONTEND
168-
169-
/*
170-
* Note that in much unlucky circumstances, the random data read from a
171-
* recycled segment can cause this routine to be called with a size
172-
* causing a hard failure at allocation. For a standby, this would cause
173-
* the instance to stop suddenly with a hard failure, preventing it to
174-
* retry fetching WAL from one of its sources which could allow it to move
175-
* on with replay without a manual restart. If the data comes from a past
176-
* recycled segment and is still valid, then the allocation may succeed
177-
* but record checks are going to fail so this would be short-lived. If
178-
* the allocation fails because of a memory shortage, then this is not a
179-
* hard failure either per the guarantee given by MCXT_ALLOC_NO_OOM.
180-
*/
181-
if (!AllocSizeIsValid(newSize))
182-
return false;
183-
184-
#endif
185-
186170
if (state->readRecordBuf)
187171
pfree(state->readRecordBuf);
188172
state->readRecordBuf=
@@ -346,15 +330,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
346330
}
347331
else
348332
{
349-
/* XXX: more validation should be done here */
350-
if (total_len<SizeOfXLogRecord)
351-
{
352-
report_invalid_record(state,
353-
"invalid record length at %X/%X: wanted %u, got %u",
354-
(uint32) (RecPtr >>32), (uint32)RecPtr,
355-
(uint32)SizeOfXLogRecord,total_len);
356-
gotoerr;
357-
}
333+
/* We'll validate the header once we have the next page. */
358334
gotheader= false;
359335
}
360336

@@ -370,17 +346,11 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
370346
assembled= true;
371347

372348
/*
373-
* Enlarge readRecordBuf as needed.
349+
* We always have space for a couple of pages, enough to validate a
350+
* boundary-spanning record header.
374351
*/
375-
if (total_len>state->readRecordBufSize&&
376-
!allocate_recordbuf(state,total_len))
377-
{
378-
/* We treat this as a "bogus data" condition */
379-
report_invalid_record(state,"record length %u at %X/%X too long",
380-
total_len,
381-
(uint32) (RecPtr >>32), (uint32)RecPtr);
382-
gotoerr;
383-
}
352+
Assert(state->readRecordBufSize >=XLOG_BLCKSZ*2);
353+
Assert(state->readRecordBufSize >=len);
384354

385355
/* Copy the first fragment of the record from the first page. */
386356
memcpy(state->readRecordBuf,
@@ -475,8 +445,37 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
475445
gotoerr;
476446
gotheader= true;
477447
}
478-
}while (gotlen<total_len);
479448

449+
/*
450+
* We might need a bigger buffer. We have validated the record
451+
* header, in the case that it split over a page boundary. We've
452+
* also cross-checked total_len against xlp_rem_len on the second
453+
* page, and verified xlp_pageaddr on both.
454+
*/
455+
Assert(gotheader);
456+
if (total_len>state->readRecordBufSize)
457+
{
458+
charsave_copy[XLOG_BLCKSZ*2];
459+
460+
/*
461+
* Save and restore the data we already had. It can't be more
462+
* than two pages.
463+
*/
464+
Assert(gotlen <=lengthof(save_copy));
465+
Assert(gotlen <=state->readRecordBufSize);
466+
memcpy(save_copy,state->readRecordBuf,gotlen);
467+
if (!allocate_recordbuf(state,total_len))
468+
{
469+
/* We treat this as a "bogus data" condition */
470+
report_invalid_record(state,"record length %u at %X/%X too long",
471+
total_len,
472+
(uint32) (RecPtr >>32), (uint32)RecPtr);
473+
gotoerr;
474+
}
475+
memcpy(state->readRecordBuf,save_copy,gotlen);
476+
buffer=state->readRecordBuf+gotlen;
477+
}
478+
}while (gotlen<total_len);
480479
Assert(gotheader);
481480

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

‎src/test/perl/TestLib.pm

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ our @EXPORT = qw(
3333
check_mode_recursive
3434
chmod_recursive
3535
check_pg_config
36+
scan_server_header
3637
system_or_bail
3738
system_log
3839
run_log
@@ -459,6 +460,39 @@ sub chmod_recursive
459460
return;
460461
}
461462

463+
# Returns an array that stores all the matches of the given regular expression
464+
# within the PostgreSQL installation's C<header_path>. This can be used to
465+
# retrieve specific value patterns from the installation's header files.
466+
subscan_server_header
467+
{
468+
my ($header_path,$regexp) =@_;
469+
470+
my ($stdout,$stderr);
471+
my$result = IPC::Run::run ['pg_config','--includedir-server' ],'>',
472+
\$stdout,'2>', \$stderr
473+
ordie"could not execute pg_config";
474+
chomp($stdout);
475+
$stdout =~s/\r$//;
476+
477+
openmy$header_h,'<',"$stdout/$header_path"ordie"$!";
478+
479+
my@match =undef;
480+
while (<$header_h>)
481+
{
482+
my$line =$_;
483+
484+
if (@match =$line =~/^$regexp/)
485+
{
486+
last;
487+
}
488+
}
489+
490+
close$header_h;
491+
die"could not find match in header$header_path\n"
492+
unless@match;
493+
return@match;
494+
}
495+
462496
# Check presence of a given regexp within pg_config.h for the installation
463497
# where tests are running, returning a match status result depending on
464498
# that.

0 commit comments

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