From 40ebfac6e22b3ae282c2bd8e7237d035666c3edf Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Wed, 23 Sep 2020 10:49:10 +0530 Subject: [PATCH 2/3] Negative block number passed to functions in pageinspect module When a negative integer is passed to get_raw_page(), bt_page_stats() and bt_page_inspect() they report an invalid block number error with a bogus number. This is because the input is casted to an unsigned integer which for negative numbers happens to be a large value. This, at the worst, is confusing. Accept the intput as a signed integer and report an error with negative input as is. Ashutosh Bapat. --- contrib/pageinspect/btreefuncs.c | 16 ++++++++++++++-- contrib/pageinspect/expected/btree.out | 4 ++++ contrib/pageinspect/expected/page.out | 4 ++++ contrib/pageinspect/rawpage.c | 18 +++++++++++++++--- contrib/pageinspect/sql/btree.sql | 2 ++ contrib/pageinspect/sql/page.sql | 2 ++ 6 files changed, 41 insertions(+), 5 deletions(-) diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c index 445605db58..494c711801 100644 --- a/contrib/pageinspect/btreefuncs.c +++ b/contrib/pageinspect/btreefuncs.c @@ -164,7 +164,7 @@ Datum bt_page_stats(PG_FUNCTION_ARGS) { text *relname = PG_GETARG_TEXT_PP(0); - uint32 blkno = PG_GETARG_UINT32(1); + int32 blkno = PG_GETARG_INT32(1); Buffer buffer; Relation rel; RangeVar *relrv; @@ -200,6 +200,12 @@ bt_page_stats(PG_FUNCTION_ARGS) if (blkno == 0) elog(ERROR, "block 0 is a meta page"); + if (blkno < 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid block number %d", + blkno))); + CHECK_RELATION_BLOCK_RANGE(rel, blkno); buffer = ReadBuffer(rel, blkno); @@ -409,7 +415,7 @@ Datum bt_page_items(PG_FUNCTION_ARGS) { text *relname = PG_GETARG_TEXT_PP(0); - uint32 blkno = PG_GETARG_UINT32(1); + int32 blkno = PG_GETARG_INT32(1); Datum result; FuncCallContext *fctx; MemoryContext mctx; @@ -420,6 +426,12 @@ bt_page_items(PG_FUNCTION_ARGS) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must be superuser to use pageinspect functions"))); + if (blkno < 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid block number %d", + blkno))); + if (SRF_IS_FIRSTCALL()) { RangeVar *relrv; diff --git a/contrib/pageinspect/expected/btree.out b/contrib/pageinspect/expected/btree.out index 17bf0c5470..1ee0a6d791 100644 --- a/contrib/pageinspect/expected/btree.out +++ b/contrib/pageinspect/expected/btree.out @@ -32,6 +32,8 @@ btpo_flags | 3 SELECT * FROM bt_page_stats('test1_a_idx', 2); ERROR: block number out of range +SELECT * FROM bt_page_stats('test1_a_idx', -1); +ERROR: invalid block number -1 SELECT * FROM bt_page_items('test1_a_idx', 0); ERROR: block 0 is a meta page SELECT * FROM bt_page_items('test1_a_idx', 1); @@ -48,6 +50,8 @@ tids | SELECT * FROM bt_page_items('test1_a_idx', 2); ERROR: block number out of range +SELECT * FROM bt_page_items('test1_a_idx', -1); +ERROR: invalid block number -1 SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 0)); ERROR: block is a meta page SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 1)); diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out index b6aea0124b..66de360eed 100644 --- a/contrib/pageinspect/expected/page.out +++ b/contrib/pageinspect/expected/page.out @@ -12,6 +12,8 @@ SELECT octet_length(get_raw_page('test1', 'main', 0)) AS main_0; SELECT octet_length(get_raw_page('test1', 'main', 1)) AS main_1; ERROR: block number 1 is out of range for relation "test1" +SELECT octet_length(get_raw_page('test1', 'main', -1)) AS main_1; +ERROR: invalid block number -1 SELECT octet_length(get_raw_page('test1', 'fsm', 0)) AS fsm_0; fsm_0 ------- @@ -49,6 +51,8 @@ SELECT pagesize, version FROM page_header(get_raw_page('test1', 0)); 8192 | 4 (1 row) +SELECT pagesize, version FROM page_header(get_raw_page('test1', -1)); +ERROR: invalid block number -1 SELECT page_checksum(get_raw_page('test1', 0), 0) IS NOT NULL AS silly_checksum_test; silly_checksum_test --------------------- diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c index c0181506a5..9035a50342 100644 --- a/contrib/pageinspect/rawpage.c +++ b/contrib/pageinspect/rawpage.c @@ -46,7 +46,7 @@ Datum get_raw_page(PG_FUNCTION_ARGS) { text *relname = PG_GETARG_TEXT_PP(0); - uint32 blkno = PG_GETARG_UINT32(1); + int32 blkno = PG_GETARG_INT32(1); bytea *raw_page; /* @@ -59,7 +59,13 @@ get_raw_page(PG_FUNCTION_ARGS) (errmsg("wrong number of arguments to get_raw_page()"), errhint("Run the updated pageinspect.sql script."))); - raw_page = get_raw_page_internal(relname, MAIN_FORKNUM, blkno); + if (blkno < 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid block number %d", + blkno))); + + raw_page = get_raw_page_internal(relname, MAIN_FORKNUM, (uint32) blkno); PG_RETURN_BYTEA_P(raw_page); } @@ -76,10 +82,16 @@ get_raw_page_fork(PG_FUNCTION_ARGS) { text *relname = PG_GETARG_TEXT_PP(0); text *forkname = PG_GETARG_TEXT_PP(1); - uint32 blkno = PG_GETARG_UINT32(2); + int32 blkno = PG_GETARG_INT32(2); bytea *raw_page; ForkNumber forknum; + if (blkno < 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid block number %d", + blkno))); + forknum = forkname_to_number(text_to_cstring(forkname)); raw_page = get_raw_page_internal(relname, forknum, blkno); diff --git a/contrib/pageinspect/sql/btree.sql b/contrib/pageinspect/sql/btree.sql index 8eac64c7b3..ef581f4838 100644 --- a/contrib/pageinspect/sql/btree.sql +++ b/contrib/pageinspect/sql/btree.sql @@ -9,10 +9,12 @@ SELECT * FROM bt_metap('test1_a_idx'); SELECT * FROM bt_page_stats('test1_a_idx', 0); SELECT * FROM bt_page_stats('test1_a_idx', 1); SELECT * FROM bt_page_stats('test1_a_idx', 2); +SELECT * FROM bt_page_stats('test1_a_idx', -1); SELECT * FROM bt_page_items('test1_a_idx', 0); SELECT * FROM bt_page_items('test1_a_idx', 1); SELECT * FROM bt_page_items('test1_a_idx', 2); +SELECT * FROM bt_page_items('test1_a_idx', -1); SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 0)); SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 1)); diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql index bd049aeb24..05170e34fd 100644 --- a/contrib/pageinspect/sql/page.sql +++ b/contrib/pageinspect/sql/page.sql @@ -10,6 +10,7 @@ VACUUM test1; -- set up FSM SELECT octet_length(get_raw_page('test1', 'main', 0)) AS main_0; SELECT octet_length(get_raw_page('test1', 'main', 1)) AS main_1; +SELECT octet_length(get_raw_page('test1', 'main', -1)) AS main_1; SELECT octet_length(get_raw_page('test1', 'fsm', 0)) AS fsm_0; SELECT octet_length(get_raw_page('test1', 'fsm', 1)) AS fsm_1; @@ -23,6 +24,7 @@ SELECT octet_length(get_raw_page('test1', 'xxx', 0)); SELECT get_raw_page('test1', 0) = get_raw_page('test1', 'main', 0); SELECT pagesize, version FROM page_header(get_raw_page('test1', 0)); +SELECT pagesize, version FROM page_header(get_raw_page('test1', -1)); SELECT page_checksum(get_raw_page('test1', 0), 0) IS NOT NULL AS silly_checksum_test; -- 2.17.1