Hi Rahul,

Thank you very much for the thorough review and testing -- the crash-recovery 
matrix (pglz/lz4/zstd/off + consistency checking) and especially the 
crafted-record repro with pg_waldump --save-fullpage crashing without the patch 
are super convincing. And thanks for confirming the back-patch safety argument.

v2 attached, addressing all your points:

Commit message now says uncompressed images are affected too. You are right -- 
there hole_length is derived as BLCKSZ - bimg_len and hole_offset was unchecked 
against bimg_len; the new bound covers it (it amounts to hole_offset <= 
bimg_len there).

Rebased onto current master (%d / %X/%08X). Sorry for the stale context in v1 
-- it was made against an older tree. v2 applies with git am.

3-4. Dropped the runtime re-check in RestoreBlockImage() in favor of Assert()s, 
as you suggested. Since DecodeXLogRecord() is the only filler of 
DecodedBkpBlock, it cannot fail, and this also resolves the wrong format and 
the duplicate message text.

About back-patching: agreed, thanks for spelling out the XLogRecordAssemble() 
condition -- no valid WAL can trip the new check.

About the ReadRecPtr vs decoded-record LSN issue: good catch, thank you for 
noticing while testing. And yes, please send that as a separate patch.

About the frontend test: attached as 0002, since you said you'd like to see it. 
It is a small src/test/modules/test_xlogreader harness that builds hole-image 
records in memory (valid CRC, compressed + uncompressed) and feeds them to 
DecodeXLogRecord(), so it checks rejection happens at decode time, before 
RestoreBlockImage() can touch the page:
- valid compressed hole, hole ending exactly at BLCKSZ,
- the BUG #19599 shape (hole_offset + hole_length > BLCKSZ), one byte past the 
page, hole_offset past the page,
- valid uncompressed hole and uncompressed hole_offset past the image.

With v2 all valid cases are accepted and all malformed ones are rejected with 
the existing BKPIMAGE_HAS_HOLE set message. I kept it as a separate patch to 
keep v2 small -- happy to squash if you prefer. And I'd still love to see your 
crafting script, it sounds more end-to-end than mine.

Thanks again,
  Yuriy

Attachment: 0001-Reject-WAL-block-images-whose-hole-does-not-fit-v2.patch
Description: 0001-Reject-WAL-block-images-whose-hole-does-not-fit-v2.patch

Attachment: 0002-Add-test_xlogreader-for-malformed-FPI-hole-geometry.patch
Description: 0002-Add-test_xlogreader-for-malformed-FPI-hole-geometry.patch

Reply via email to