Skip to content

Commit

Permalink
pageinspect: Fix crash with gist_page_items()
Browse files Browse the repository at this point in the history
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
  • Loading branch information
michaelpq committed Mar 2, 2023
1 parent 1a9356f commit 5ad63ee
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 56 deletions.
8 changes: 5 additions & 3 deletions contrib/pageinspect/expected/brin.out
Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions contrib/pageinspect/expected/gist.out
Expand Up @@ -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
Expand Down
82 changes: 40 additions & 42 deletions contrib/pageinspect/gistfuncs.c
Expand Up @@ -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)))
Expand All @@ -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)
Expand All @@ -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));
Expand All @@ -124,7 +141,6 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
Tuplestorestate *tupstore;
MemoryContext oldcontext;
Page page;
GISTPageOpaque opaq;
OffsetNumber offset;
OffsetNumber maxoff = InvalidOffsetNumber;

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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))
{
Expand Down
8 changes: 5 additions & 3 deletions contrib/pageinspect/sql/brin.sql
Expand Up @@ -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));
Expand Down
10 changes: 6 additions & 4 deletions contrib/pageinspect/sql/gist.sql
Expand Up @@ -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);
Expand Down

0 comments on commit 5ad63ee

Please sign in to comment.