Re: BUG #16527: Valgrind detects an invalid read in brin_revmap_data with non-index page

On Sat, Jul 04, 2020 at 10:04:25PM +0200, Tomas Vondra wrote:
>On Sat, Jul 04, 2020 at 04:00:00PM +0000, PG Bug reporting form wrote:
>>The following bug has been logged on the website:
>>
>>Bug reference:      16527
>>Logged by:          Alexander Lakhin
>>Email address:      exclusion@gmail.com
>>PostgreSQL version: 13beta2
>>Operating system:   Ubuntu 20.04
>>Description:
>>
>>Running the following query (based on contrib/pageinspect/sql/brin.sql)
>>under valgrind:
>>CREATE EXTENSION pageinspect;
>>CREATE TABLE test1 (a int, b text);
>>INSERT INTO test1 VALUES (1, 'one');
>>SELECT * FROM brin_revmap_data(get_raw_page('test1', 0));
>>
>>leads to a memory access error:
>>==00:00:00:12.518 934833== Invalid read of size 2
>>==00:00:00:12.518 934833==    at 0x4865AE1: verify_brin_page
>>(brinfuncs.c:107)
>> ...
>>}
>>2020-07-04 17:57:55.915 MSK [934833] ERROR:  page is not a BRIN page of type
>>"revmap"
>>2020-07-04 17:57:55.915 MSK [934833] DETAIL:  Expected special type
>>0000f092, got 00007f7f.
>
>Hmmm, the 7f7f kinda seems like the pattern we use for randomizing
>allocated/freed memory. So I thought maybe we're not initializing the
>memory properly, or maybe freeing it too early. But I was getting
>different patterns, and the reality is way simpler:
>
>
>test=# SELECT * FROM page_header(get_raw_page('test1', 0));
>    lsn    | checksum | flags | lower | upper | special | pagesize | version | prune_xid
>-----------+----------+-------+-------+-------+---------+----------+---------+-----------
> 0/15BBE80 |        0 |     4 |    28 |  8160 |    8192 |     8192 |       4 |         0
>(1 row)
>
>So the page actually does not have any special part, which is where the
>type is supposed to be stored. So the BrinPageType probably ends up
>reading whatever is immediately after the page. Interesting.
>
>It might be worth adding an assert to check the PageGetSpecialPointer
>result is actually within the page.
>

FWIW at first I was puzzled why we're not seeing other failures when a
page unexpectedly does not have a special chunk at the end, but the
reason is pretty simple - the page comes from a table, not from a BRIN
index. I initially missed this detail.

So there probably needs to be some sort of check (in verify_brin_page or
somewhere close) that the page has just the right pg_special value, and
fail with ERROR if not. An assert seems insufficient in this case.

I also wonder if other similar pageinspect cases with mismatched data
(page read from an index, passed to a heap-related functions etc.) have
the same issue.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services