Skip to content

Commit 5ad63ee

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 1a9356f commit 5ad63ee

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
@@ -66,14 +66,16 @@ SELECT itemoffset, ctid, itemlen FROM gist_page_items_bytea(get_raw_page('test_g
6666
7 | (7,65535) | 40
6767
(7 rows)
6868

69-
-- Failure with non-GiST index.
69+
-- Suppress the DETAIL message, to allow the tests to work across various
70+
-- page sizes and architectures.
71+
\set VERBOSITY terse
72+
-- Failures with non-GiST index.
7073
CREATE INDEX test_gist_btree on test_gist(t);
7174
SELECT gist_page_items(get_raw_page('test_gist_btree', 0), 'test_gist_btree');
7275
ERROR: "test_gist_btree" is not a GiST index
76+
SELECT gist_page_items(get_raw_page('test_gist_btree', 0), 'test_gist_idx');
77+
ERROR: input page is not a valid GiST page
7378
-- Failure with various modes.
74-
-- Suppress the DETAIL message, to allow the tests to work across various
75-
-- page sizes and architectures.
76-
\set VERBOSITY terse
7779
-- invalid page size
7880
SELECT gist_page_items_bytea('aaa'::bytea);
7981
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));
@@ -124,7 +141,6 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
124141
Tuplestorestate *tupstore;
125142
MemoryContext oldcontext;
126143
Page page;
127-
GISTPageOpaque opaq;
128144
OffsetNumber offset;
129145
OffsetNumber maxoff = InvalidOffsetNumber;
130146

@@ -157,29 +173,11 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
157173

158174
MemoryContextSwitchTo(oldcontext);
159175

160-
page = get_page_from_raw(raw_page);
176+
page = verify_gist_page(raw_page);
161177

162178
if (PageIsNew(page))
163179
PG_RETURN_NULL();
164180

165-
/* verify the special space has the expected size */
166-
if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
167-
ereport(ERROR,
168-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
169-
errmsg("input page is not a valid %s page", "GiST"),
170-
errdetail("Expected special size %d, got %d.",
171-
(int) MAXALIGN(sizeof(GISTPageOpaqueData)),
172-
(int) PageGetSpecialSize(page))));
173-
174-
opaq = (GISTPageOpaque) PageGetSpecialPointer(page);
175-
if (opaq->gist_page_id != GIST_PAGE_ID)
176-
ereport(ERROR,
177-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
178-
errmsg("input page is not a valid %s page", "GiST"),
179-
errdetail("Expected %08x, got %08x.",
180-
GIST_PAGE_ID,
181-
opaq->gist_page_id)));
182-
183181
/* Avoid bogus PageGetMaxOffsetNumber() call with deleted pages */
184182
if (GistPageIsDeleted(page))
185183
elog(NOTICE, "page is deleted");
@@ -276,7 +274,7 @@ gist_page_items(PG_FUNCTION_ARGS)
276274
errmsg("\"%s\" is not a %s index",
277275
RelationGetRelationName(indexRel), "GiST")));
278276

279-
page = get_page_from_raw(raw_page);
277+
page = verify_gist_page(raw_page);
280278

281279
if (PageIsNew(page))
282280
{

‎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 (endianess), 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)