Skip to content

Commit af8a8eb

Browse files
michaelpqmichaelpq
committed
pageinspect: Fix handling of page sizes and AM types
This commit fixes a set of issues related to the use of the SQL functions in this module when the caller is able to pass down raw page data as input argument: - The page size check was fuzzy in a couple of places, sometimes looking after only a sub-range, but what we are looking for is an exact match on BLCKSZ. After considering a few options here, I have settled down to do a generalization of get_page_from_raw(). Most of the SQL functions already used that, and this is not strictly required if not accessing an 8-byte-wide value from a raw page, but this feels safer in the long run for alignment-picky environment, particularly if a code path begins to access such values. This also reduces the number of strings that need to be translated. - The BRIN function brin_page_items() uses a Relation but it did not check the access method of the opened index, potentially leading to crashes. All the other functions in need of a Relation already did that. - Some code paths could fail on elog(), but we should to use ereport() for failures that can be triggered by the user. Tests are added to stress all the cases that are fixed as of this commit, with some junk raw pages (\set VERBOSITY ensures that this works across all page sizes) and unexpected index types when functions open relations. Author: Michael Paquier, Justin Prysby Discussion: https://postgr.es/m/20220218030020.GA1137@telsasoft.com Backpatch-through: 10
1 parent 45a469e commit af8a8eb

File tree

15 files changed

+142
-67
lines changed

15 files changed

+142
-67
lines changed

‎contrib/pageinspect/brinfuncs.c

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "access/brin_revmap.h"
1919
#include "access/brin_tuple.h"
2020
#include "catalog/index.h"
21+
#include "catalog/pg_am_d.h"
2122
#include "catalog/pg_type.h"
2223
#include "funcapi.h"
2324
#include "lib/stringinfo.h"
@@ -33,6 +34,8 @@ PG_FUNCTION_INFO_V1(brin_page_items);
3334
PG_FUNCTION_INFO_V1(brin_metapage_info);
3435
PG_FUNCTION_INFO_V1(brin_revmap_data);
3536

37+
#define IS_BRIN(r) ((r)->rd_rel->relam == BRIN_AM_OID)
38+
3639
typedef struct brin_column_state
3740
{
3841
int nstored;
@@ -47,23 +50,15 @@ Datum
4750
brin_page_type(PG_FUNCTION_ARGS)
4851
{
4952
bytea *raw_page = PG_GETARG_BYTEA_P(0);
50-
Page page = VARDATA(raw_page);
51-
int raw_page_size;
53+
Page page;
5254
char *type;
5355

5456
if (!superuser())
5557
ereport(ERROR,
5658
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
5759
(errmsg("must be superuser to use raw page functions"))));
5860

59-
raw_page_size = VARSIZE(raw_page) - VARHDRSZ;
60-
61-
if (raw_page_size != BLCKSZ)
62-
ereport(ERROR,
63-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
64-
errmsg("input page too small"),
65-
errdetail("Expected size %d, got %d",
66-
BLCKSZ, raw_page_size)));
61+
page = get_page_from_raw(raw_page);
6762

6863
switch (BrinPageType(page))
6964
{
@@ -91,19 +86,7 @@ brin_page_type(PG_FUNCTION_ARGS)
9186
static Page
9287
verify_brin_page(bytea *raw_page, uint16 type, const char *strtype)
9388
{
94-
Page page;
95-
int raw_page_size;
96-
97-
raw_page_size = VARSIZE(raw_page) - VARHDRSZ;
98-
99-
if (raw_page_size != BLCKSZ)
100-
ereport(ERROR,
101-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
102-
errmsg("input page too small"),
103-
errdetail("Expected size %d, got %d",
104-
BLCKSZ, raw_page_size)));
105-
106-
page = VARDATA(raw_page);
89+
Page page = get_page_from_raw(raw_page);
10790

10891
/* verify the special space says this page is what we want */
10992
if (BrinPageType(page) != type)
@@ -171,6 +154,13 @@ brin_page_items(PG_FUNCTION_ARGS)
171154
MemoryContextSwitchTo(oldcontext);
172155

173156
indexRel = index_open(indexRelid, AccessShareLock);
157+
158+
if (!IS_BRIN(indexRel))
159+
ereport(ERROR,
160+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
161+
errmsg("\"%s\" is not a %s index",
162+
RelationGetRelationName(indexRel), "BRIN")));
163+
174164
bdesc = brin_build_desc(indexRel);
175165

176166
/* minimally verify the page we got */

‎contrib/pageinspect/btreefuncs.c

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,10 @@ bt_page_stats(PG_FUNCTION_ARGS)
182182
rel = relation_openrv(relrv, AccessShareLock);
183183

184184
if (!IS_INDEX(rel) || !IS_BTREE(rel))
185-
elog(ERROR, "relation \"%s\" is not a btree index",
186-
RelationGetRelationName(rel));
185+
ereport(ERROR,
186+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
187+
errmsg("\"%s\" is not a %s index",
188+
RelationGetRelationName(rel), "btree")));
187189

188190
/*
189191
* Reject attempts to read non-local temporary relations; we would be
@@ -336,8 +338,10 @@ bt_page_items(PG_FUNCTION_ARGS)
336338
rel = relation_openrv(relrv, AccessShareLock);
337339

338340
if (!IS_INDEX(rel) || !IS_BTREE(rel))
339-
elog(ERROR, "relation \"%s\" is not a btree index",
340-
RelationGetRelationName(rel));
341+
ereport(ERROR,
342+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
343+
errmsg("\"%s\" is not a %s index",
344+
RelationGetRelationName(rel), "btree")));
341345

342346
/*
343347
* Reject attempts to read non-local temporary relations; we would be
@@ -425,7 +429,6 @@ bt_page_items_bytea(PG_FUNCTION_ARGS)
425429
Datum result;
426430
FuncCallContext *fctx;
427431
struct user_args *uargs;
428-
int raw_page_size;
429432

430433
if (!superuser())
431434
ereport(ERROR,
@@ -438,19 +441,12 @@ bt_page_items_bytea(PG_FUNCTION_ARGS)
438441
MemoryContext mctx;
439442
TupleDesc tupleDesc;
440443

441-
raw_page_size = VARSIZE(raw_page) - VARHDRSZ;
442-
443-
if (raw_page_size < SizeOfPageHeaderData)
444-
ereport(ERROR,
445-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
446-
errmsg("input page too small (%d bytes)", raw_page_size)));
447-
448444
fctx = SRF_FIRSTCALL_INIT();
449445
mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
450446

451447
uargs = palloc(sizeof(struct user_args));
452448

453-
uargs->page = VARDATA(raw_page);
449+
uargs->page = get_page_from_raw(raw_page);
454450

455451
uargs->offset = FirstOffsetNumber;
456452

@@ -526,8 +522,10 @@ bt_metap(PG_FUNCTION_ARGS)
526522
rel = relation_openrv(relrv, AccessShareLock);
527523

528524
if (!IS_INDEX(rel) || !IS_BTREE(rel))
529-
elog(ERROR, "relation \"%s\" is not a btree index",
530-
RelationGetRelationName(rel));
525+
ereport(ERROR,
526+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
527+
errmsg("\"%s\" is not a %s index",
528+
RelationGetRelationName(rel), "btree")));
531529

532530
/*
533531
* Reject attempts to read non-local temporary relations; we would be

‎contrib/pageinspect/expected/brin.out

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,8 @@ 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.
52+
CREATE INDEX test1_a_btree ON test1 (a);
53+
SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_btree');
54+
ERROR: "test1_a_btree" is not a BRIN index
5155
DROP TABLE test1;

‎contrib/pageinspect/expected/btree.out

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,19 @@ data | 01 00 00 00 00 00 00 01
5757

5858
SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 2));
5959
ERROR: block number 2 is out of range for relation "test1_a_idx"
60+
-- Failure when using a non-btree index.
61+
CREATE INDEX test1_a_hash ON test1 USING hash(a);
62+
SELECT bt_metap('test1_a_hash');
63+
ERROR: "test1_a_hash" is not a btree index
64+
SELECT bt_page_stats('test1_a_hash', 0);
65+
ERROR: "test1_a_hash" is not a btree index
66+
SELECT bt_page_items('test1_a_hash', 0);
67+
ERROR: "test1_a_hash" is not a btree index
68+
-- Failure with incorrect page size
69+
-- Suppress the DETAIL message, to allow the tests to work across various
70+
-- page sizes.
71+
\set VERBOSITY terse
72+
SELECT bt_page_items('aaa'::bytea);
73+
ERROR: invalid page size
74+
\set VERBOSITY default
6075
DROP TABLE test1;

‎contrib/pageinspect/expected/gin.out

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,14 @@ FROM gin_leafpage_items(get_raw_page('test1_y_idx',
3535
-[ RECORD 1 ]
3636
?column? | t
3737

38+
-- Failure with incorrect page size
39+
-- Suppress the DETAIL message, to allow the tests to work across various
40+
-- page sizes.
41+
\set VERBOSITY terse
42+
SELECT gin_leafpage_items('aaa'::bytea);
43+
ERROR: invalid page size
44+
SELECT gin_metapage_info('bbb'::bytea);
45+
ERROR: invalid page size
46+
SELECT gin_page_opaque_info('ccc'::bytea);
47+
ERROR: invalid page size
48+
\set VERBOSITY default

‎contrib/pageinspect/expected/hash.out

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,4 +159,21 @@ SELECT * FROM hash_page_items(get_raw_page('test_hash_a_idx', 4));
159159

160160
SELECT * FROM hash_page_items(get_raw_page('test_hash_a_idx', 5));
161161
ERROR: page is not a hash bucket or overflow page
162+
-- Failure with non-hash index
163+
CREATE INDEX test_hash_a_btree ON test_hash USING btree (a);
164+
SELECT hash_bitmap_info('test_hash_a_btree', 0);
165+
ERROR: "test_hash_a_btree" is not a hash index
166+
-- Failure with incorrect page size
167+
-- Suppress the DETAIL message, to allow the tests to work across various
168+
-- page sizes.
169+
\set VERBOSITY terse
170+
SELECT hash_metapage_info('aaa'::bytea);
171+
ERROR: invalid page size
172+
SELECT hash_page_items('bbb'::bytea);
173+
ERROR: invalid page size
174+
SELECT hash_page_stats('ccc'::bytea);
175+
ERROR: invalid page size
176+
SELECT hash_page_type('ddd'::bytea);
177+
ERROR: invalid page size
178+
\set VERBOSITY default
162179
DROP TABLE test_hash;

‎contrib/pageinspect/expected/page.out

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,14 @@ select tuple_data_split('test8'::regclass, t_data, t_infomask, t_infomask2, t_bi
113113
(1 row)
114114

115115
drop table test8;
116+
-- Failure with incorrect page size
117+
-- Suppress the DETAIL message, to allow the tests to work across various
118+
-- page sizes.
119+
\set VERBOSITY terse
120+
SELECT fsm_page_contents('aaa'::bytea);
121+
ERROR: invalid page size
122+
SELECT page_checksum('bbb'::bytea, 0);
123+
ERROR: invalid page size
124+
SELECT page_header('ccc'::bytea);
125+
ERROR: invalid page size
126+
\set VERBOSITY default

‎contrib/pageinspect/fsmfuncs.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ fsm_page_contents(PG_FUNCTION_ARGS)
3737
{
3838
bytea *raw_page = PG_GETARG_BYTEA_P(0);
3939
StringInfoData sinfo;
40+
Page page;
4041
FSMPage fsmpage;
4142
int i;
4243

@@ -45,7 +46,8 @@ fsm_page_contents(PG_FUNCTION_ARGS)
4546
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
4647
(errmsg("must be superuser to use raw page functions"))));
4748

48-
fsmpage = (FSMPage) PageGetContents(VARDATA(raw_page));
49+
page = get_page_from_raw(raw_page);
50+
fsmpage = (FSMPage) PageGetContents(page);
4951

5052
initStringInfo(&sinfo);
5153

‎contrib/pageinspect/hashfuncs.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -420,8 +420,10 @@ hash_bitmap_info(PG_FUNCTION_ARGS)
420420
indexRel = index_open(indexRelid, AccessShareLock);
421421

422422
if (!IS_HASH(indexRel))
423-
elog(ERROR, "relation \"%s\" is not a hash index",
424-
RelationGetRelationName(indexRel));
423+
ereport(ERROR,
424+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
425+
errmsg("\"%s\" is not a %s index",
426+
RelationGetRelationName(indexRel), "hash")));
425427

426428
if (RELATION_IS_OTHER_TEMP(indexRel))
427429
ereport(ERROR,

‎contrib/pageinspect/rawpage.c

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,6 @@ Datum
219219
page_header(PG_FUNCTION_ARGS)
220220
{
221221
bytea *raw_page = PG_GETARG_BYTEA_P(0);
222-
int raw_page_size;
223222

224223
TupleDesc tupdesc;
225224

@@ -236,18 +235,7 @@ page_header(PG_FUNCTION_ARGS)
236235
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
237236
(errmsg("must be superuser to use raw page functions"))));
238237

239-
raw_page_size = VARSIZE(raw_page) - VARHDRSZ;
240-
241-
/*
242-
* Check that enough data was supplied, so that we don't try to access
243-
* fields outside the supplied buffer.
244-
*/
245-
if (raw_page_size < SizeOfPageHeaderData)
246-
ereport(ERROR,
247-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
248-
errmsg("input page too small (%d bytes)", raw_page_size)));
249-
250-
page = (PageHeader) VARDATA(raw_page);
238+
page = (PageHeader) get_page_from_raw(raw_page);
251239

252240
/* Build a tuple descriptor for our result type */
253241
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
@@ -300,25 +288,14 @@ page_checksum(PG_FUNCTION_ARGS)
300288
{
301289
bytea *raw_page = PG_GETARG_BYTEA_P(0);
302290
uint32 blkno = PG_GETARG_INT32(1);
303-
int raw_page_size;
304-
PageHeader page;
291+
Page page;
305292

306293
if (!superuser())
307294
ereport(ERROR,
308295
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
309296
(errmsg("must be superuser to use raw page functions"))));
310297

311-
raw_page_size = VARSIZE(raw_page) - VARHDRSZ;
312-
313-
/*
314-
* Check that the supplied page is of the right size.
315-
*/
316-
if (raw_page_size != BLCKSZ)
317-
ereport(ERROR,
318-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
319-
errmsg("incorrect size of input page (%d bytes)", raw_page_size)));
320-
321-
page = (PageHeader) VARDATA(raw_page);
298+
page = get_page_from_raw(raw_page);
322299

323300
PG_RETURN_INT16(pg_checksum_page((char *) page, blkno));
324301
}

‎contrib/pageinspect/sql/brin.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ 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.
19+
CREATE INDEX test1_a_btree ON test1 (a);
20+
SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_btree');
21+
1822
DROP TABLE test1;

‎contrib/pageinspect/sql/btree.sql

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,17 @@ SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 0));
1818
SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 1));
1919
SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 2));
2020

21+
-- Failure when using a non-btree index.
22+
CREATE INDEX test1_a_hash ON test1 USING hash(a);
23+
SELECT bt_metap('test1_a_hash');
24+
SELECT bt_page_stats('test1_a_hash', 0);
25+
SELECT bt_page_items('test1_a_hash', 0);
26+
27+
-- Failure with incorrect page size
28+
-- Suppress the DETAIL message, to allow the tests to work across various
29+
-- page sizes.
30+
\set VERBOSITY terse
31+
SELECT bt_page_items('aaa'::bytea);
32+
\set VERBOSITY default
33+
2134
DROP TABLE test1;

‎contrib/pageinspect/sql/gin.sql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,12 @@ SELECT COUNT(*) > 0
1717
FROM gin_leafpage_items(get_raw_page('test1_y_idx',
1818
(pg_relation_size('test1_y_idx') /
1919
current_setting('block_size')::bigint)::int - 1));
20+
21+
-- Failure with incorrect page size
22+
-- Suppress the DETAIL message, to allow the tests to work across various
23+
-- page sizes.
24+
\set VERBOSITY terse
25+
SELECT gin_leafpage_items('aaa'::bytea);
26+
SELECT gin_metapage_info('bbb'::bytea);
27+
SELECT gin_page_opaque_info('ccc'::bytea);
28+
\set VERBOSITY default

‎contrib/pageinspect/sql/hash.sql

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,18 @@ SELECT * FROM hash_page_items(get_raw_page('test_hash_a_idx', 3));
7676
SELECT * FROM hash_page_items(get_raw_page('test_hash_a_idx', 4));
7777
SELECT * FROM hash_page_items(get_raw_page('test_hash_a_idx', 5));
7878

79+
-- Failure with non-hash index
80+
CREATE INDEX test_hash_a_btree ON test_hash USING btree (a);
81+
SELECT hash_bitmap_info('test_hash_a_btree', 0);
82+
83+
-- Failure with incorrect page size
84+
-- Suppress the DETAIL message, to allow the tests to work across various
85+
-- page sizes.
86+
\set VERBOSITY terse
87+
SELECT hash_metapage_info('aaa'::bytea);
88+
SELECT hash_page_items('bbb'::bytea);
89+
SELECT hash_page_stats('ccc'::bytea);
90+
SELECT hash_page_type('ddd'::bytea);
91+
\set VERBOSITY default
7992

8093
DROP TABLE test_hash;

‎contrib/pageinspect/sql/page.sql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,12 @@ select t_bits, t_data from heap_page_items(get_raw_page('test8', 0));
5252
select tuple_data_split('test8'::regclass, t_data, t_infomask, t_infomask2, t_bits)
5353
from heap_page_items(get_raw_page('test8', 0));
5454
drop table test8;
55+
56+
-- Failure with incorrect page size
57+
-- Suppress the DETAIL message, to allow the tests to work across various
58+
-- page sizes.
59+
\set VERBOSITY terse
60+
SELECT fsm_page_contents('aaa'::bytea);
61+
SELECT page_checksum('bbb'::bytea, 0);
62+
SELECT page_header('ccc'::bytea);
63+
\set VERBOSITY default

0 commit comments

Comments
 (0)