From 5ad63eee13e70eeff9659bcee024e8249b6bf68c Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 2 Mar 2023 14:03:21 +0900 Subject: [PATCH] pageinspect: Fix crash with gist_page_items() Attempting to use this function with a raw page not coming from a GiST index would cause a crash, as it was missing the same sanity checks as gist_page_items_bytea(). This slightly refactors the code so as all the basic validation checks for GiST pages are done in a single routine, in the same fashion as the pageinspect functions for hash and BRIN. This fixes an issue similar to 076f4d9. A test is added to stress for this case. While on it, I have added a similar test for brin_page_items() with a combination make of a valid GiST index and a raw btree page. This one was already protected, but it was not tested. Reported-by: Egor Chindyaskin Author: Dmitry Koval Discussion: https://postgr.es/m/17815-fc4a2d3b74705703@postgresql.org Backpatch-through: 14 --- contrib/pageinspect/expected/brin.out | 8 ++- contrib/pageinspect/expected/gist.out | 10 ++-- contrib/pageinspect/gistfuncs.c | 82 +++++++++++++-------------- contrib/pageinspect/sql/brin.sql | 8 ++- contrib/pageinspect/sql/gist.sql | 10 ++-- 5 files changed, 62 insertions(+), 56 deletions(-) diff --git a/contrib/pageinspect/expected/brin.out b/contrib/pageinspect/expected/brin.out index d19cdc3b957f8..e12fbeb47741d 100644 --- a/contrib/pageinspect/expected/brin.out +++ b/contrib/pageinspect/expected/brin.out @@ -48,12 +48,14 @@ SELECT * FROM brin_page_items(get_raw_page('test1_a_idx', 2), 'test1_a_idx') 1 | 0 | 1 | f | f | f | {1 .. 1} (1 row) --- Failure for non-BRIN index. +-- Mask DETAIL messages as these are not portable across architectures. +\set VERBOSITY terse +-- Failures for non-BRIN index. CREATE INDEX test1_a_btree ON test1 (a); SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_btree'); ERROR: "test1_a_btree" is not a BRIN index --- Mask DETAIL messages as these are not portable across architectures. -\set VERBOSITY terse +SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_idx'); +ERROR: input page is not a valid BRIN page -- Invalid special area size SELECT brin_page_type(get_raw_page('test1', 0)); ERROR: input page is not a valid BRIN page diff --git a/contrib/pageinspect/expected/gist.out b/contrib/pageinspect/expected/gist.out index 469fc5eabf67d..3f2e720d0244c 100644 --- a/contrib/pageinspect/expected/gist.out +++ b/contrib/pageinspect/expected/gist.out @@ -66,14 +66,16 @@ SELECT itemoffset, ctid, itemlen FROM gist_page_items_bytea(get_raw_page('test_g 7 | (7,65535) | 40 (7 rows) --- Failure with non-GiST index. +-- Suppress the DETAIL message, to allow the tests to work across various +-- page sizes and architectures. +\set VERBOSITY terse +-- Failures with non-GiST index. CREATE INDEX test_gist_btree on test_gist(t); SELECT gist_page_items(get_raw_page('test_gist_btree', 0), 'test_gist_btree'); ERROR: "test_gist_btree" is not a GiST index +SELECT gist_page_items(get_raw_page('test_gist_btree', 0), 'test_gist_idx'); +ERROR: input page is not a valid GiST page -- Failure with various modes. --- Suppress the DETAIL message, to allow the tests to work across various --- page sizes and architectures. -\set VERBOSITY terse -- invalid page size SELECT gist_page_items_bytea('aaa'::bytea); ERROR: invalid page size diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c index d1c3c321f830f..0ae8f7459c165 100644 --- a/contrib/pageinspect/gistfuncs.c +++ b/contrib/pageinspect/gistfuncs.c @@ -34,29 +34,20 @@ PG_FUNCTION_INFO_V1(gist_page_items_bytea); #define ItemPointerGetDatum(X) PointerGetDatum(X) -Datum -gist_page_opaque_info(PG_FUNCTION_ARGS) +static Page verify_gist_page(bytea *raw_page); + +/* + * Verify that the given bytea contains a GIST page or die in the attempt. + * A pointer to the page is returned. + */ +static Page +verify_gist_page(bytea *raw_page) { - bytea *raw_page = PG_GETARG_BYTEA_P(0); - TupleDesc tupdesc; - Page page; + Page page = get_page_from_raw(raw_page); GISTPageOpaque opaq; - HeapTuple resultTuple; - Datum values[4]; - bool nulls[4]; - Datum flags[16]; - int nflags = 0; - uint16 flagbits; - - if (!superuser()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to use raw page functions"))); - - page = get_page_from_raw(raw_page); if (PageIsNew(page)) - PG_RETURN_NULL(); + return page; /* verify the special space has the expected size */ if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData))) @@ -76,12 +67,38 @@ gist_page_opaque_info(PG_FUNCTION_ARGS) GIST_PAGE_ID, opaq->gist_page_id))); + return page; +} + +Datum +gist_page_opaque_info(PG_FUNCTION_ARGS) +{ + bytea *raw_page = PG_GETARG_BYTEA_P(0); + TupleDesc tupdesc; + Page page; + HeapTuple resultTuple; + Datum values[4]; + bool nulls[4]; + Datum flags[16]; + int nflags = 0; + uint16 flagbits; + + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to use raw page functions"))); + + page = verify_gist_page(raw_page); + + if (PageIsNew(page)) + PG_RETURN_NULL(); + /* Build a tuple descriptor for our result type */ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) elog(ERROR, "return type must be a row type"); /* Convert the flags bitmask to an array of human-readable names */ - flagbits = opaq->flags; + flagbits = GistPageGetOpaque(page)->flags; if (flagbits & F_LEAF) flags[nflags++] = CStringGetTextDatum("leaf"); if (flagbits & F_DELETED) @@ -103,7 +120,7 @@ gist_page_opaque_info(PG_FUNCTION_ARGS) values[0] = LSNGetDatum(PageGetLSN(page)); values[1] = LSNGetDatum(GistPageGetNSN(page)); - values[2] = Int64GetDatum(opaq->rightlink); + values[2] = Int64GetDatum(GistPageGetOpaque(page)->rightlink); values[3] = PointerGetDatum(construct_array(flags, nflags, TEXTOID, -1, false, TYPALIGN_INT)); @@ -124,7 +141,6 @@ gist_page_items_bytea(PG_FUNCTION_ARGS) Tuplestorestate *tupstore; MemoryContext oldcontext; Page page; - GISTPageOpaque opaq; OffsetNumber offset; OffsetNumber maxoff = InvalidOffsetNumber; @@ -157,29 +173,11 @@ gist_page_items_bytea(PG_FUNCTION_ARGS) MemoryContextSwitchTo(oldcontext); - page = get_page_from_raw(raw_page); + page = verify_gist_page(raw_page); if (PageIsNew(page)) PG_RETURN_NULL(); - /* verify the special space has the expected size */ - if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData))) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("input page is not a valid %s page", "GiST"), - errdetail("Expected special size %d, got %d.", - (int) MAXALIGN(sizeof(GISTPageOpaqueData)), - (int) PageGetSpecialSize(page)))); - - opaq = (GISTPageOpaque) PageGetSpecialPointer(page); - if (opaq->gist_page_id != GIST_PAGE_ID) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("input page is not a valid %s page", "GiST"), - errdetail("Expected %08x, got %08x.", - GIST_PAGE_ID, - opaq->gist_page_id))); - /* Avoid bogus PageGetMaxOffsetNumber() call with deleted pages */ if (GistPageIsDeleted(page)) elog(NOTICE, "page is deleted"); @@ -276,7 +274,7 @@ gist_page_items(PG_FUNCTION_ARGS) errmsg("\"%s\" is not a %s index", RelationGetRelationName(indexRel), "GiST"))); - page = get_page_from_raw(raw_page); + page = verify_gist_page(raw_page); if (PageIsNew(page)) { diff --git a/contrib/pageinspect/sql/brin.sql b/contrib/pageinspect/sql/brin.sql index 45098c1ef5e4b..96b4645187e0e 100644 --- a/contrib/pageinspect/sql/brin.sql +++ b/contrib/pageinspect/sql/brin.sql @@ -15,12 +15,14 @@ SELECT * FROM brin_revmap_data(get_raw_page('test1_a_idx', 1)) LIMIT 5; SELECT * FROM brin_page_items(get_raw_page('test1_a_idx', 2), 'test1_a_idx') ORDER BY blknum, attnum LIMIT 5; --- Failure for non-BRIN index. +-- Mask DETAIL messages as these are not portable across architectures. +\set VERBOSITY terse + +-- Failures for non-BRIN index. CREATE INDEX test1_a_btree ON test1 (a); SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_btree'); +SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_idx'); --- Mask DETAIL messages as these are not portable across architectures. -\set VERBOSITY terse -- Invalid special area size SELECT brin_page_type(get_raw_page('test1', 0)); SELECT * FROM brin_metapage_info(get_raw_page('test1', 0)); diff --git a/contrib/pageinspect/sql/gist.sql b/contrib/pageinspect/sql/gist.sql index ee46e09053e2a..963d5d40a3c7f 100644 --- a/contrib/pageinspect/sql/gist.sql +++ b/contrib/pageinspect/sql/gist.sql @@ -26,14 +26,16 @@ SELECT * FROM gist_page_items(get_raw_page('test_gist_idx', 1), 'test_gist_idx') -- platform-dependent (endianess), so omit the actual key data from the output. SELECT itemoffset, ctid, itemlen FROM gist_page_items_bytea(get_raw_page('test_gist_idx', 0)); --- Failure with non-GiST index. +-- Suppress the DETAIL message, to allow the tests to work across various +-- page sizes and architectures. +\set VERBOSITY terse + +-- Failures with non-GiST index. CREATE INDEX test_gist_btree on test_gist(t); SELECT gist_page_items(get_raw_page('test_gist_btree', 0), 'test_gist_btree'); +SELECT gist_page_items(get_raw_page('test_gist_btree', 0), 'test_gist_idx'); -- Failure with various modes. --- Suppress the DETAIL message, to allow the tests to work across various --- page sizes and architectures. -\set VERBOSITY terse -- invalid page size SELECT gist_page_items_bytea('aaa'::bytea); SELECT gist_page_items('aaa'::bytea, 'test_gist_idx'::regclass);