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

diff --git a/contrib/pageinspect/expected/brin.out b/contrib/pageinspect/expected/brin.out
index d19cdc3b957f8fe1910a3e7e66679331504a17c7..e12fbeb47741d3370ac216388d36119acec37495 100644 (file)
--- 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 a51bded0cb6e7ce6957ea193e5bee53753078676..460bef3037e6c33874bc9f5c42c579294985dbba 100644 (file)
--- a/contrib/pageinspect/expected/gist.out
+++ b/contrib/pageinspect/expected/gist.out
@@ -64,14 +64,16 @@ SELECT itemoffset, ctid, itemlen FROM gist_page_items_bytea(get_raw_page('test_g
           6 | (6,65535) |      40
 (6 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 100697814dcf47fc9858907a4c41729cd24ba3ed..3dca7f1318f14795c8e3a17135bdfacd7383bbb3 100644 (file)
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -32,29 +32,20 @@ PG_FUNCTION_INFO_V1(gist_page_items_bytea);
 #define IS_GIST(r) ((r)->rd_rel->relam == GIST_AM_OID)
 
 
-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)))
@@ -74,12 +65,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)
@@ -101,7 +118,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_builtin(flags, nflags, TEXTOID));
 
    /* Build and return the result tuple. */
@@ -116,7 +133,6 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
    bytea      *raw_page = PG_GETARG_BYTEA_P(0);
    ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
    Page        page;
-   GISTPageOpaque opaq;
    OffsetNumber offset;
    OffsetNumber maxoff = InvalidOffsetNumber;
 
@@ -127,29 +143,11 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
 
    InitMaterializedSRF(fcinfo, 0);
 
-   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 = GistPageGetOpaque(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");
@@ -220,7 +218,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 45098c1ef5e4baa34e5f4e9dc0ad79e59c846dbc..96b4645187e0e75e5cf665c32420f61d7aec94c7 100644 (file)
--- 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 1edf2b307f61aab923dc3e67308fd17c36a86dbd..4787b784a464246a54b71509b5edbc4b3c500123 100644 (file)
--- 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 (endianness), 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);