Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit be75363

Browse files
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 d7056bc commit be75363

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
@@ -32,29 +32,20 @@ PG_FUNCTION_INFO_V1(gist_page_items_bytea);
3232
#define IS_GIST(r) ((r)->rd_rel->relam == GIST_AM_OID)
3333

3434

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

5647
if (PageIsNew(page))
57-
PG_RETURN_NULL();
48+
return page;
5849

5950
/* verify the special space has the expected size */
6051
if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
@@ -74,12 +65,38 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
7465
GIST_PAGE_ID,
7566
opaq->gist_page_id)));
7667

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

8198
/* Convert the flags bitmask to an array of human-readable names */
82-
flagbits = opaq->flags;
99+
flagbits = GistPageGetOpaque(page)->flags;
83100
if (flagbits & F_LEAF)
84101
flags[nflags++] = CStringGetTextDatum("leaf");
85102
if (flagbits & F_DELETED)
@@ -101,7 +118,7 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
101118

102119
values[0] = LSNGetDatum(PageGetLSN(page));
103120
values[1] = LSNGetDatum(GistPageGetNSN(page));
104-
values[2] = Int64GetDatum(opaq->rightlink);
121+
values[2] = Int64GetDatum(GistPageGetOpaque(page)->rightlink);
105122
values[3] = PointerGetDatum(construct_array_builtin(flags, nflags, TEXTOID));
106123

107124
/* Build and return the result tuple. */
@@ -116,7 +133,6 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
116133
bytea *raw_page = PG_GETARG_BYTEA_P(0);
117134
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
118135
Page page;
119-
GISTPageOpaque opaq;
120136
OffsetNumber offset;
121137
OffsetNumber maxoff = InvalidOffsetNumber;
122138

@@ -127,29 +143,11 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
127143

128144
InitMaterializedSRF(fcinfo, 0);
129145

130-
page = get_page_from_raw(raw_page);
146+
page = verify_gist_page(raw_page);
131147

132148
if (PageIsNew(page))
133149
PG_RETURN_NULL();
134150

135-
/* verify the special space has the expected size */
136-
if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
137-
ereport(ERROR,
138-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
139-
errmsg("input page is not a valid %s page", "GiST"),
140-
errdetail("Expected special size %d, got %d.",
141-
(int) MAXALIGN(sizeof(GISTPageOpaqueData)),
142-
(int) PageGetSpecialSize(page))));
143-
144-
opaq = GistPageGetOpaque(page);
145-
if (opaq->gist_page_id != GIST_PAGE_ID)
146-
ereport(ERROR,
147-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
148-
errmsg("input page is not a valid %s page", "GiST"),
149-
errdetail("Expected %08x, got %08x.",
150-
GIST_PAGE_ID,
151-
opaq->gist_page_id)));
152-
153151
/* Avoid bogus PageGetMaxOffsetNumber() call with deleted pages */
154152
if (GistPageIsDeleted(page))
155153
elog(NOTICE, "page is deleted");
@@ -220,7 +218,7 @@ gist_page_items(PG_FUNCTION_ARGS)
220218
errmsg("\"%s\" is not a %s index",
221219
RelationGetRelationName(indexRel), "GiST")));
222220

223-
page = get_page_from_raw(raw_page);
221+
page = verify_gist_page(raw_page);
224222

225223
if (PageIsNew(page))
226224
{

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)