Skip to content

Commit 9d41ecf

Browse files
michaelpqmichaelpq
committed
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
1 parent eae0913 commit 9d41ecf

File tree

5 files changed

+62
-56
lines changed

5 files changed

+62
-56
lines changed

‎contrib/pageinspect/expected/brin.out

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,14 @@ SELECT * FROM brin_page_items(get_raw_page('test1_a_idx', 2), 'test1_a_idx')
4848
1 | 0 | 1 | f | f | f | {1 .. 1}
4949
(1 row)
5050

51-
-- Failure for non-BRIN index.
51+
-- Mask DETAIL messages as these are not portable across architectures.
52+
\set VERBOSITY terse
53+
-- Failures for non-BRIN index.
5254
CREATE INDEX test1_a_btree ON test1 (a);
5355
SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_btree');
5456
ERROR: "test1_a_btree" is not a BRIN index
55-
-- Mask DETAIL messages as these are not portable across architectures.
56-
\set VERBOSITY terse
57+
SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_idx');
58+
ERROR: input page is not a valid BRIN page
5759
-- Invalid special area size
5860
SELECT brin_page_type(get_raw_page('test1', 0));
5961
ERROR: input page is not a valid BRIN page

‎contrib/pageinspect/expected/gist.out

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,14 +64,16 @@ SELECT itemoffset, ctid, itemlen FROM gist_page_items_bytea(get_raw_page('test_g
6464
6 | (6,65535) | 40
6565
(6 rows)
6666

67-
-- Failure with non-GiST index.
67+
-- Suppress the DETAIL message, to allow the tests to work across various
68+
-- page sizes and architectures.
69+
\set VERBOSITY terse
70+
-- Failures with non-GiST index.
6871
CREATE INDEX test_gist_btree on test_gist(t);
6972
SELECT gist_page_items(get_raw_page('test_gist_btree', 0), 'test_gist_btree');
7073
ERROR: "test_gist_btree" is not a GiST index
74+
SELECT gist_page_items(get_raw_page('test_gist_btree', 0), 'test_gist_idx');
75+
ERROR: input page is not a valid GiST page
7176
-- Failure with various modes.
72-
-- Suppress the DETAIL message, to allow the tests to work across various
73-
-- page sizes and architectures.
74-
\set VERBOSITY terse
7577
-- invalid page size
7678
SELECT gist_page_items_bytea('aaa'::bytea);
7779
ERROR: invalid page size

‎contrib/pageinspect/gistfuncs.c

Lines changed: 40 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -34,29 +34,20 @@ PG_FUNCTION_INFO_V1(gist_page_items_bytea);
3434
#define ItemPointerGetDatum(X) PointerGetDatum(X)
3535

3636

37-
Datum
38-
gist_page_opaque_info(PG_FUNCTION_ARGS)
37+
static Page verify_gist_page(bytea *raw_page);
38+
39+
/*
40+
* Verify that the given bytea contains a GIST page or die in the attempt.
41+
* A pointer to the page is returned.
42+
*/
43+
static Page
44+
verify_gist_page(bytea *raw_page)
3945
{
40-
bytea *raw_page = PG_GETARG_BYTEA_P(0);
41-
TupleDesc tupdesc;
42-
Page page;
46+
Page page = get_page_from_raw(raw_page);
4347
GISTPageOpaque opaq;
44-
HeapTuple resultTuple;
45-
Datum values[4];
46-
bool nulls[4];
47-
Datum flags[16];
48-
int nflags = 0;
49-
uint16 flagbits;
50-
51-
if (!superuser())
52-
ereport(ERROR,
53-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
54-
errmsg("must be superuser to use raw page functions")));
55-
56-
page = get_page_from_raw(raw_page);
5748

5849
if (PageIsNew(page))
59-
PG_RETURN_NULL();
50+
return page;
6051

6152
/* verify the special space has the expected size */
6253
if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
@@ -76,12 +67,38 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
7667
GIST_PAGE_ID,
7768
opaq->gist_page_id)));
7869

70+
return page;
71+
}
72+
73+
Datum
74+
gist_page_opaque_info(PG_FUNCTION_ARGS)
75+
{
76+
bytea *raw_page = PG_GETARG_BYTEA_P(0);
77+
TupleDesc tupdesc;
78+
Page page;
79+
HeapTuple resultTuple;
80+
Datum values[4];
81+
bool nulls[4];
82+
Datum flags[16];
83+
int nflags = 0;
84+
uint16 flagbits;
85+
86+
if (!superuser())
87+
ereport(ERROR,
88+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
89+
errmsg("must be superuser to use raw page functions")));
90+
91+
page = verify_gist_page(raw_page);
92+
93+
if (PageIsNew(page))
94+
PG_RETURN_NULL();
95+
7996
/* Build a tuple descriptor for our result type */
8097
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
8198
elog(ERROR, "return type must be a row type");
8299

83100
/* Convert the flags bitmask to an array of human-readable names */
84-
flagbits = opaq->flags;
101+
flagbits = GistPageGetOpaque(page)->flags;
85102
if (flagbits & F_LEAF)
86103
flags[nflags++] = CStringGetTextDatum("leaf");
87104
if (flagbits & F_DELETED)
@@ -103,7 +120,7 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
103120

104121
values[0] = LSNGetDatum(PageGetLSN(page));
105122
values[1] = LSNGetDatum(GistPageGetNSN(page));
106-
values[2] = Int64GetDatum(opaq->rightlink);
123+
values[2] = Int64GetDatum(GistPageGetOpaque(page)->rightlink);
107124
values[3] = PointerGetDatum(construct_array(flags, nflags,
108125
TEXTOID,
109126
-1, false, TYPALIGN_INT));
@@ -120,7 +137,6 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
120137
bytea *raw_page = PG_GETARG_BYTEA_P(0);
121138
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
122139
Page page;
123-
GISTPageOpaque opaq;
124140
OffsetNumber offset;
125141
OffsetNumber maxoff = InvalidOffsetNumber;
126142

@@ -131,29 +147,11 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
131147

132148
InitMaterializedSRF(fcinfo, 0);
133149

134-
page = get_page_from_raw(raw_page);
150+
page = verify_gist_page(raw_page);
135151

136152
if (PageIsNew(page))
137153
PG_RETURN_NULL();
138154

139-
/* verify the special space has the expected size */
140-
if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
141-
ereport(ERROR,
142-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
143-
errmsg("input page is not a valid %s page", "GiST"),
144-
errdetail("Expected special size %d, got %d.",
145-
(int) MAXALIGN(sizeof(GISTPageOpaqueData)),
146-
(int) PageGetSpecialSize(page))));
147-
148-
opaq = GistPageGetOpaque(page);
149-
if (opaq->gist_page_id != GIST_PAGE_ID)
150-
ereport(ERROR,
151-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
152-
errmsg("input page is not a valid %s page", "GiST"),
153-
errdetail("Expected %08x, got %08x.",
154-
GIST_PAGE_ID,
155-
opaq->gist_page_id)));
156-
157155
/* Avoid bogus PageGetMaxOffsetNumber() call with deleted pages */
158156
if (GistPageIsDeleted(page))
159157
elog(NOTICE, "page is deleted");
@@ -224,7 +222,7 @@ gist_page_items(PG_FUNCTION_ARGS)
224222
errmsg("\"%s\" is not a %s index",
225223
RelationGetRelationName(indexRel), "GiST")));
226224

227-
page = get_page_from_raw(raw_page);
225+
page = verify_gist_page(raw_page);
228226

229227
if (PageIsNew(page))
230228
{

‎contrib/pageinspect/sql/brin.sql

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@ SELECT * FROM brin_revmap_data(get_raw_page('test1_a_idx', 1)) LIMIT 5;
1515
SELECT * FROM brin_page_items(get_raw_page('test1_a_idx', 2), 'test1_a_idx')
1616
ORDER BY blknum, attnum LIMIT 5;
1717

18-
-- Failure for non-BRIN index.
18+
-- Mask DETAIL messages as these are not portable across architectures.
19+
\set VERBOSITY terse
20+
21+
-- Failures for non-BRIN index.
1922
CREATE INDEX test1_a_btree ON test1 (a);
2023
SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_btree');
24+
SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_idx');
2125

22-
-- Mask DETAIL messages as these are not portable across architectures.
23-
\set VERBOSITY terse
2426
-- Invalid special area size
2527
SELECT brin_page_type(get_raw_page('test1', 0));
2628
SELECT * FROM brin_metapage_info(get_raw_page('test1', 0));

‎contrib/pageinspect/sql/gist.sql

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,16 @@ SELECT * FROM gist_page_items(get_raw_page('test_gist_idx', 1), 'test_gist_idx')
2626
-- platform-dependent (endianness), so omit the actual key data from the output.
2727
SELECT itemoffset, ctid, itemlen FROM gist_page_items_bytea(get_raw_page('test_gist_idx', 0));
2828

29-
-- Failure with non-GiST index.
29+
-- Suppress the DETAIL message, to allow the tests to work across various
30+
-- page sizes and architectures.
31+
\set VERBOSITY terse
32+
33+
-- Failures with non-GiST index.
3034
CREATE INDEX test_gist_btree on test_gist(t);
3135
SELECT gist_page_items(get_raw_page('test_gist_btree', 0), 'test_gist_btree');
36+
SELECT gist_page_items(get_raw_page('test_gist_btree', 0), 'test_gist_idx');
3237

3338
-- Failure with various modes.
34-
-- Suppress the DETAIL message, to allow the tests to work across various
35-
-- page sizes and architectures.
36-
\set VERBOSITY terse
3739
-- invalid page size
3840
SELECT gist_page_items_bytea('aaa'::bytea);
3941
SELECT gist_page_items('aaa'::bytea, 'test_gist_idx'::regclass);

0 commit comments

Comments
 (0)