Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 51fed14

Browse files
committed
Don't get confused if a WAL partial record header has xl_tot_len == 0.
If a WAL record header was split across pages, but xl_tot_len was 0, we would get confused and conclude that we had already read the whole record, and proceed to CRC check it. That can lead to a crash in RecordIsValid(), which isn't careful to not read beyond end-of-record, as defined by xl_tot_len. Add an explicit sanity check for xl_tot_len <= SizeOfXlogRecord. Also, make RecordIsValid() more robust by checking in each step that it doesn't try to access memory beyond end of record, even if a length field in the record's or a backup block's header is bogus. Per report and analysis by Tom Lane.
1 parent 9b2a237 commit 51fed14

File tree

1 file changed

+36
-2
lines changed
  • src/backend/access/transam

1 file changed

+36
-2
lines changed

src/backend/access/transam/xlog.c

+36-2
Original file line numberDiff line numberDiff line change
@@ -3707,8 +3707,17 @@ RecordIsValid(XLogRecord *record, XLogRecPtr recptr, int emode)
37073707
uint32 len = record->xl_len;
37083708
BkpBlock bkpb;
37093709
char *blk;
3710+
char *recend = (char *) record + record->xl_tot_len;
37103711

37113712
/* First the rmgr data */
3713+
if (XLogRecGetData(record) + len > recend)
3714+
{
3715+
/* ValidXLogRecordHeader() should've caught this already... */
3716+
ereport(emode_for_corrupt_record(emode, recptr),
3717+
(errmsg("invalid record length at %X/%X",
3718+
(uint32) (recptr >> 32), (uint32) recptr)));
3719+
return false;
3720+
}
37123721
INIT_CRC32(crc);
37133722
COMP_CRC32(crc, XLogRecGetData(record), len);
37143723

@@ -3721,7 +3730,15 @@ RecordIsValid(XLogRecord *record, XLogRecPtr recptr, int emode)
37213730
if (!(record->xl_info & XLR_SET_BKP_BLOCK(i)))
37223731
continue;
37233732

3733+
if (blk + sizeof(BkpBlock) > recend)
3734+
{
3735+
ereport(emode_for_corrupt_record(emode, recptr),
3736+
(errmsg("incorrect backup block size in record at %X/%X",
3737+
(uint32) (recptr >> 32), (uint32) recptr)));
3738+
return false;
3739+
}
37243740
memcpy(&bkpb, blk, sizeof(BkpBlock));
3741+
37253742
if (bkpb.hole_offset + bkpb.hole_length > BLCKSZ)
37263743
{
37273744
ereport(emode_for_corrupt_record(emode, recptr),
@@ -3730,6 +3747,14 @@ RecordIsValid(XLogRecord *record, XLogRecPtr recptr, int emode)
37303747
return false;
37313748
}
37323749
blen = sizeof(BkpBlock) + BLCKSZ - bkpb.hole_length;
3750+
3751+
if (blk + blen > recend)
3752+
{
3753+
ereport(emode_for_corrupt_record(emode, recptr),
3754+
(errmsg("invalid backup block size in record at %X/%X",
3755+
(uint32) (recptr >> 32), (uint32) recptr)));
3756+
return false;
3757+
}
37333758
COMP_CRC32(crc, blk, blen);
37343759
blk += blen;
37353760
}
@@ -3879,8 +3904,8 @@ ReadRecord(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt)
38793904

38803905
/*
38813906
* If the whole record header is on this page, validate it immediately.
3882-
* Otherwise validate it after reading the rest of the header from next
3883-
* page.
3907+
* Otherwise only do a basic sanity check on xl_tot_len, and validate the
3908+
* rest of the header after reading it from the next page.
38843909
*/
38853910
if (targetRecOff <= XLOG_BLCKSZ - SizeOfXLogRecord)
38863911
{
@@ -3889,7 +3914,16 @@ ReadRecord(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt)
38893914
gotheader = true;
38903915
}
38913916
else
3917+
{
3918+
if (total_len < SizeOfXLogRecord)
3919+
{
3920+
ereport(emode_for_corrupt_record(emode, *RecPtr),
3921+
(errmsg("invalid record length at %X/%X",
3922+
(uint32) ((*RecPtr) >> 32), (uint32) *RecPtr)));
3923+
goto next_record_is_invalid;
3924+
}
38923925
gotheader = false;
3926+
}
38933927

38943928
/*
38953929
* Allocate or enlarge readRecordBuf as needed. To avoid useless small

0 commit comments

Comments
 (0)