Hi, Michael reported this as BUG #19599. He wasn't sure it was a real issue, but I think the missing check is still worth fixing. Postgres never writes a hole that doesn't fit in the page, but DecodeXLogRecord() will happily accept one from a corrupt or hand-built record, and RestoreBlockImage() then uses those fields as memcpy/MemSet lengths.
What decode checks today is only non-zero values:
> if ((blk->bimg_info & BKPIMAGE_HAS_HOLE) &&
> (blk->hole_offset == 0 ||
> blk->hole_length == 0 ||
> blk->bimg_len == BLCKSZ))
It never asks whether the hole actually fits in the page. Both fields
are uint16s taken from the record, so this gets through:
hole_offset = 8000
hole_length = 8000 /* 16000 > BLCKSZ */
bimg_len = 16
bimg_info = HAS_HOLE | APPLY | COMPRESS_PGLZ
That's specifically a compressed image, because that's the only shape
where hole_length is stored in the WAL rather than computed. The CRC
can still be valid. After that, RestoreBlockImage() does:
memcpy(page, ptr, hole_offset);
MemSet(page + hole_offset, 0, hole_length);
memcpy(..., BLCKSZ - (hole_offset + hole_length));
With the numbers above, the first memcpy already runs off the end of an
8kB page, and the last length underflows to a huge size_t. The
decompressors have the same problem: they are told the output buffer is
BLCKSZ - hole_length bytes.
I've attached the patch that adds the missing bound to that existing HAS_HOLE
check:
hole_offset > BLCKSZ ||
hole_length > BLCKSZ - hole_offset
(subtraction rather than addition, so the two uint16s can't overflow.)
The same check is repeated at the start of RestoreBlockImage(), before
decompression or memcpy. A bad hole still uses the existing HAS_HOLE
error message, I didn't add a new one.
I also have a small frontend test that builds this record in memory (valid
header and CRC, compressed image, bad hole) and feeds it to
DecodeXLogRecord(). Happy to send that if it's useful, I left it out of
this mail so the patch stays small.
Does this look like the right approach?
I'd also like to hear whether it's worth back-patching.
Thanks,
Yuriy Grigoryev
0001-Reject-WAL-block-images-whose-hole-does-not-fit.patch
Description: 0001-Reject-WAL-block-images-whose-hole-does-not-fit.patch
