From 03cacee5c74d02c8fcb39ccb7cb8c2bc416b536c Mon Sep 17 00:00:00 2001 From: Shihao Zhong Date: Mon, 31 Aug 2026 00:28:41 -0400 Subject: [PATCH 2/2] pageinspect: validate page headers, and BRIN and GIN page contents Walking a page whose header is corrupt could address memory outside the page image. Verify the header first, and bound BRIN line pointers and GIN posting lists in the same way as the other access methods. --- contrib/pageinspect/brinfuncs.c | 11 +++++++++ contrib/pageinspect/btreefuncs.c | 1 + contrib/pageinspect/expected/brin.out | 4 ++++ contrib/pageinspect/expected/gin.out | 7 ++++++ contrib/pageinspect/expected/page.out | 6 +++++ contrib/pageinspect/ginfuncs.c | 13 ++++++++++- contrib/pageinspect/gistfuncs.c | 2 ++ contrib/pageinspect/hashfuncs.c | 2 ++ contrib/pageinspect/heapfuncs.c | 1 + contrib/pageinspect/pageinspect.h | 1 + contrib/pageinspect/rawpage.c | 32 +++++++++++++++++++++++++++ contrib/pageinspect/sql/brin.sql | 4 ++++ contrib/pageinspect/sql/gin.sql | 7 ++++++ contrib/pageinspect/sql/page.sql | 6 +++++ 14 files changed, 96 insertions(+), 1 deletion(-) diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c index c64124b5b02..ba5aaf73bde 100644 --- a/contrib/pageinspect/brinfuncs.c +++ b/contrib/pageinspect/brinfuncs.c @@ -95,6 +95,8 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype) { Page page = get_page_from_raw(raw_page); + verify_page_header(page); + if (PageIsNew(page)) return page; @@ -232,6 +234,15 @@ brin_page_items(PG_FUNCTION_ARGS) itemId = PageGetItemId(page, offset); if (ItemIdIsUsed(itemId)) { + /* Check that the line pointer and tuple lie within the page. */ + if (ItemIdGetOffset(itemId) != MAXALIGN(ItemIdGetOffset(itemId)) || + ItemIdGetLength(itemId) < SizeOfBrinTuple || + ItemIdGetOffset(itemId) + ItemIdGetLength(itemId) > BLCKSZ) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid line pointer at offset %u in BRIN page", + offset))); + dtup = brin_deform_tuple(bdesc, (BrinTuple *) PageGetItem(page, itemId), NULL); diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c index acecd00eae5..5c19e835a52 100644 --- a/contrib/pageinspect/btreefuncs.c +++ b/contrib/pageinspect/btreefuncs.c @@ -768,6 +768,7 @@ bt_page_items_bytea(PG_FUNCTION_ARGS) uargs = palloc_object(ua_page_items); uargs->page = get_page_from_raw(raw_page); + verify_page_header(uargs->page); if (PageIsNew(uargs->page)) { diff --git a/contrib/pageinspect/expected/brin.out b/contrib/pageinspect/expected/brin.out index b6a43bbb01b..f9ad809dcc1 100644 --- a/contrib/pageinspect/expected/brin.out +++ b/contrib/pageinspect/expected/brin.out @@ -106,6 +106,10 @@ SELECT (COUNT(*) = (SELECT relpages FROM pg_class WHERE relname = 'test2')) AS r t (1 row) +-- A corrupt line pointer must be reported, not read out of bounds. All-ones is +-- an invalid (out-of-range, unaligned) line pointer on any architecture. +SELECT brin_page_items(set_byte(set_byte(set_byte(set_byte(get_raw_page('test1_a_idx', 2), 24, 255), 25, 255), 26, 255), 27, 255), 'test1_a_idx'); +ERROR: invalid line pointer at offset 1 in BRIN page DROP TABLE test1; DROP TABLE test2; -- Test that parallel index build produces the same BRIN index as serial build. diff --git a/contrib/pageinspect/expected/gin.out b/contrib/pageinspect/expected/gin.out index ff1da6a5a17..dc5a41a110b 100644 --- a/contrib/pageinspect/expected/gin.out +++ b/contrib/pageinspect/expected/gin.out @@ -35,6 +35,13 @@ FROM gin_leafpage_items(get_raw_page('test1_y_idx', -[ RECORD 1 ] ?column? | t +-- A posting list that claims to be longer than the page must be reported, +-- rather than read out of bounds. The first list's nbytes is at offset 38. +SELECT gin_leafpage_items(set_byte(set_byte(get_raw_page('test1_y_idx', + (pg_relation_size('test1_y_idx') / + current_setting('block_size')::bigint)::int - 1), + 38, 255), 39, 255)); +ERROR: invalid posting list in GIN data leaf page -- Failure with various modes. -- Suppress the DETAIL message, to allow the tests to work across various -- page sizes and architectures. diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out index fcf19c5ca5a..ddfbbdc4d53 100644 --- a/contrib/pageinspect/expected/page.out +++ b/contrib/pageinspect/expected/page.out @@ -175,6 +175,12 @@ SELECT * FROM heap_tuple_infomask_flags(x'0010'::int, 0); {HEAP_XMAX_KEYSHR_LOCK} | {} (1 row) +-- A corrupt page header must be rejected, rather than being used to walk off +-- the end of the page. pd_lower is at offset 12. +\set VERBOSITY terse +SELECT heap_page_items(set_byte(set_byte(get_raw_page('test1', 0), 12, 255), 13, 255)); +ERROR: invalid page header +\set VERBOSITY default DROP TABLE test1; -- check that using any of these functions with a partitioned table or index -- would fail diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c index 058ad52b671..5f9b58ea18a 100644 --- a/contrib/pageinspect/ginfuncs.c +++ b/contrib/pageinspect/ginfuncs.c @@ -198,6 +198,7 @@ gin_leafpage_items(PG_FUNCTION_ARGS) mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx); page = get_page_from_raw(raw_page); + verify_page_header(page); if (PageIsNew(page)) { @@ -243,7 +244,7 @@ gin_leafpage_items(PG_FUNCTION_ARGS) fctx = SRF_PERCALL_SETUP(); inter_call_data = fctx->user_fctx; - if (inter_call_data->seg != inter_call_data->lastseg) + if (inter_call_data->seg < inter_call_data->lastseg) { GinPostingList *cur = inter_call_data->seg; HeapTuple resultTuple; @@ -255,6 +256,16 @@ gin_leafpage_items(PG_FUNCTION_ARGS) ItemPointer tids; Datum *tids_datum; + /* + * Check that the segment, and the data it claims to hold, lie within + * the posting lists area. + */ + if ((char *) cur + offsetof(GinPostingList, bytes) > (char *) inter_call_data->lastseg || + (char *) GinNextPostingListSegment(cur) > (char *) inter_call_data->lastseg) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid posting list in GIN data leaf page"))); + memset(nulls, 0, sizeof(nulls)); values[0] = ItemPointerGetDatum(&cur->first); diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c index 975ae24f73a..e4dead4ef2d 100644 --- a/contrib/pageinspect/gistfuncs.c +++ b/contrib/pageinspect/gistfuncs.c @@ -46,6 +46,8 @@ verify_gist_page(bytea *raw_page) Page page = get_page_from_raw(raw_page); GISTPageOpaque opaq; + verify_page_header(page); + if (PageIsNew(page)) return page; diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c index ddd4d8691f1..a6c3d381eee 100644 --- a/contrib/pageinspect/hashfuncs.c +++ b/contrib/pageinspect/hashfuncs.c @@ -61,6 +61,8 @@ verify_hash_page(bytea *raw_page, int flags) Page page = get_page_from_raw(raw_page); int pagetype = LH_UNUSED_PAGE; + verify_page_header(page); + /* Treat new pages as unused. */ if (!PageIsNew(page)) { diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c index 7be4770dc84..4acc670804c 100644 --- a/contrib/pageinspect/heapfuncs.c +++ b/contrib/pageinspect/heapfuncs.c @@ -157,6 +157,7 @@ heap_page_items(PG_FUNCTION_ARGS) inter_call_data->offset = FirstOffsetNumber; inter_call_data->page = get_page_from_raw(raw_page); + verify_page_header(inter_call_data->page); fctx->max_calls = PageGetMaxOffsetNumber(inter_call_data->page); fctx->user_fctx = inter_call_data; diff --git a/contrib/pageinspect/pageinspect.h b/contrib/pageinspect/pageinspect.h index b241fdc97b2..df74f6bae0b 100644 --- a/contrib/pageinspect/pageinspect.h +++ b/contrib/pageinspect/pageinspect.h @@ -26,5 +26,6 @@ enum pageinspect_version /* in rawpage.c */ extern Page get_page_from_raw(bytea *raw_page); +extern void verify_page_header(Page page); #endif /* _PAGEINSPECT_H_ */ diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c index d136593edb2..2217abc57d6 100644 --- a/contrib/pageinspect/rawpage.c +++ b/contrib/pageinspect/rawpage.c @@ -233,6 +233,38 @@ get_page_from_raw(bytea *raw_page) return page; } +/* + * verify_page_header + * + * Check that a page's header is self-consistent. Functions that walk the + * contents of a page must call this first: PageGetMaxOffsetNumber() and the + * other page accessors are all derived from these fields, so a corrupt header + * would make them address memory outside the page. + * + * This is deliberately not done by get_page_from_raw(), so that page_header() + * can still be used to inspect a page whose header is damaged. + */ +void +verify_page_header(Page page) +{ + PageHeader ph = (PageHeader) page; + + /* + * These are the checks PageIsVerified() makes on the header, except for + * the pd_flags test: they are the fields the page accessors derive + * addresses from. An all-zero page passes, and callers handle those. + */ + if (!(ph->pd_lower <= ph->pd_upper && + ph->pd_upper <= ph->pd_special && + ph->pd_special <= BLCKSZ && + ph->pd_special == MAXALIGN(ph->pd_special))) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid page header"), + errdetail("lower = %u, upper = %u, special = %u.", + ph->pd_lower, ph->pd_upper, ph->pd_special))); +} + /* * page_header diff --git a/contrib/pageinspect/sql/brin.sql b/contrib/pageinspect/sql/brin.sql index 4edf17b5a45..bdb56719a51 100644 --- a/contrib/pageinspect/sql/brin.sql +++ b/contrib/pageinspect/sql/brin.sql @@ -52,6 +52,10 @@ SELECT (COUNT(*) = (SELECT relpages FROM pg_class WHERE relname = 'test2')) AS r (SELECT (relpages - 1) FROM pg_class WHERE relname = 'test2_a_idx')) AS pages(p), LATERAL brin_page_items(get_raw_page('test2_a_idx', p), 'test2_a_idx') AS items; +-- A corrupt line pointer must be reported, not read out of bounds. All-ones is +-- an invalid (out-of-range, unaligned) line pointer on any architecture. +SELECT brin_page_items(set_byte(set_byte(set_byte(set_byte(get_raw_page('test1_a_idx', 2), 24, 255), 25, 255), 26, 255), 27, 255), 'test1_a_idx'); + DROP TABLE test1; DROP TABLE test2; diff --git a/contrib/pageinspect/sql/gin.sql b/contrib/pageinspect/sql/gin.sql index b57466d7ebf..efd923624d6 100644 --- a/contrib/pageinspect/sql/gin.sql +++ b/contrib/pageinspect/sql/gin.sql @@ -18,6 +18,13 @@ FROM gin_leafpage_items(get_raw_page('test1_y_idx', (pg_relation_size('test1_y_idx') / current_setting('block_size')::bigint)::int - 1)); +-- A posting list that claims to be longer than the page must be reported, +-- rather than read out of bounds. The first list's nbytes is at offset 38. +SELECT gin_leafpage_items(set_byte(set_byte(get_raw_page('test1_y_idx', + (pg_relation_size('test1_y_idx') / + current_setting('block_size')::bigint)::int - 1), + 38, 255), 39, 255)); + -- Failure with various modes. -- Suppress the DETAIL message, to allow the tests to work across various -- page sizes and architectures. diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql index c75fe1147f6..b34c9a13f36 100644 --- a/contrib/pageinspect/sql/page.sql +++ b/contrib/pageinspect/sql/page.sql @@ -62,6 +62,12 @@ SELECT * FROM heap_tuple_infomask_flags(0, 0); -- no combined flags SELECT * FROM heap_tuple_infomask_flags(x'0010'::int, 0); +-- A corrupt page header must be rejected, rather than being used to walk off +-- the end of the page. pd_lower is at offset 12. +\set VERBOSITY terse +SELECT heap_page_items(set_byte(set_byte(get_raw_page('test1', 0), 12, 255), 13, 255)); +\set VERBOSITY default + DROP TABLE test1; -- check that using any of these functions with a partitioned table or index -- 2.37.1 (Apple Git-137.1)