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(at)gmail(dot)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