On Monday, August 31st, 2026 at 8:13 PM, shihao zhong <[email protected]>
wrote:
> Hi hackers,
>
> I went looking at the other access methods and found a worse case than the
> one I first reported, so here is a two-patch series.
>
> 0001 is what I sent before, unchanged: GiST, B-tree and hash page_items()
> follow a line pointer after only checking ItemIdIsValid(), which just tests
> that the pointer isn't NULL -- and it never is.
>
> 0002 is the new part. Nothing checks the page header at all, and pd_lower
> is what PageGetMaxOffsetNumber() is computed from, so two bytes are enough
> to walk off the end of the line pointer array.
>
>
> That includes heap_page_items() crash on a corrupt page:
> CREATE TABLE t AS SELECT i FROM generate_series(1,200) i;
> -- pd_lower is at offset 12
> SELECT
> heap_page_items(set_byte(set_byte(get_raw_page('t',0),12,255),13,255));
> ==ERROR: AddressSanitizer: heap-buffer-overflow READ of size 4
> #0 heap_page_items heapfuncs.c:188
>
> So 0002 adds verify_page_header(), using the same checks PageIsVerified()
> makes on the header. I didn't call PageIsVerified() itself because it also
> verifies the checksum, and there is no block number for a raw bytea; happy
> to factor those checks into a shared helper instead, if that's preferred.
> I also kept it out of get_page_from_raw(), so page_header() can still show
> you have a damaged header.
> While I was there, BRIN and GIN turned out to have the same problem as
> 0001, so 0002 fixes those too.
>
> Thanks,
> Shihao
Hey Shihao,
While testing this patch I hit a related case it does not yet cover: on a
crafted page, bt_page_items() can still abort the backend on an assertion,
on master and with this patch applied.
CREATE EXTENSION pageinspect;
CREATE TABLE t (a int);
INSERT INTO t SELECT g FROM generate_series(1, 2000) g;
CREATE INDEX t_bt ON t (a);
SELECT count(*) FROM bt_page_items(set_byte(get_raw_page('t_bt', 1), 26,
245));
-- TRAP: failed Assert("!ItemIdIsDead(id)"), btreefuncs.c
Block 1 is the first leaf of a 2000-row index and not the rightmost, so its
first item is the high key, a pivot tuple. Bytes 24..27 are that line
pointer's ItemIdData {lp_off:15, lp_flags:2, lp_len:15}; writing 0xF5 to
byte 26 sets lp_flags to LP_DEAD while leaving lp_off and lp_len usable.
bt_page_print_tuples() then reaches
/* LP_DEAD bit can never be set for pivot tuples, so show a NULL there */
if (!ispivottuple)
values[j++] = BoolGetDatum(ItemIdIsDead(id));
else
{
Assert(!ItemIdIsDead(id)); /* reached from a user-supplied page */
nulls[j++] = true;
}
The invariant is true of a well-formed page, but bt_page_items(bytea) takes
an arbitrary image, so this is reachable input, not a broken invariant. On a
non-assert build the flag is just misreported as NULL; on a cassert build
the backend aborts. Your patch's new line-pointer and header checks do not
catch it (the line pointer has storage and an in-range offset), so the same
call still aborts with the patch applied.
The fix fits the same shape as the other checks in these functions: report
the corrupt page instead of asserting.
diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -572,7 +572,17 @@ bt_page_print_tuples(ua_page_items *uargs)
values[j++] = BoolGetDatum(ItemIdIsDead(id));
else
{
- Assert(!ItemIdIsDead(id));
+ /*
+ * LP_DEAD is never set on a pivot tuple in a valid page, but
this
+ * function accepts an arbitrary page image, so report a
corrupt one
+ * rather than tripping an assertion (or, in a non-assert build,
+ * silently misreporting it).
+ */
+ if (ItemIdIsDead(id))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATA_CORRUPTED),
+ errmsg("invalid pivot tuple at offset
%u: line pointer marked dead",
+ offset)));
nulls[j++] = true;
}
With this applied the crafted call returns a clean error and the backend
survives; normal bt_page_items() output is unchanged. Feel free to fold it
into your series or treat it separately, whichever you prefer.
best.
-greg
