Re: [PATCH] pg_surgery: check the page header and line pointers

Hi Shihao,

The maxoffset > MaxHeapTuplesPerPage check looks correct to me and fixes
the reported access past include_this_tid[].

I think heap_force_freeze() needs one more check. PageIsVerified()
validates the basic page header ordering, but not individual line
pointers, and ItemIdIsNormal() only checks the line pointer state. An
LP_NORMAL item can still point outside the heap tuple area, while
PageGetItem() does not validate its offset or length. We then modify a
HeapTupleHeader at that address.

Before accepting an item for freezing, I think we should at least verify
that its length can hold a tuple header, its offset is MAXALIGNed, and the
tuple fits between pd_upper and pd_special. heap_force_kill() does not
dereference the tuple, so it probably should remain able to mark such a
line pointer dead.

The test's hardcoded 291 is MaxHeapTuplesPerPage only with the default
BLCKSZ. I think it's fine - a lot of tests fail with other block sizes.
More importantly, the test does not exercise the new invalid-header
branch, so it would pass without the code change on a default build.
I suggest omitting it unless we can add a test with an actually corrupted
header. See amcheck tests fro details, but I'm not sure it actually
worth the trouble here.

Thank you!

Best regards, Andrey Borodin.