Message-ID: <88b5750f-a5da-798b-e5a1-af2c3ecb9b60@gmail.com>
Date: Mon, 1 May 2023 07:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: BUG #17798: Incorrect memory access occurs when using BEFORE ROW
 UPDATE trigger
To: Peter Geoghegan <pg@bowt.ie>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-bugs@lists.postgresql.org,
 Richard Guo <guofenglinux@gmail.com>, Andres Freund <andres@anarazel.de>
References: <17798-0907404928dcf0dd@postgresql.org>
 <87fe6c28-80b2-f709-a8db-d105e5240e39@gmail.com>
 <CAMbWs4_pg7ab=OJF2CGLzs=noWEjujerxfo92sm3RL9ce=ogsQ@mail.gmail.com>
 <CAMbWs48FHhuuuWbxygO4Wey7ahdaSwRjzJODKXx7=GFsge_aLg@mail.gmail.com>
 <f5ff446f-72e2-44f7-80c6-4256cf7caeb3@gmail.com>
 <a4a8cf37-e9d5-3c58-fc2e-d217cf66f315@gmail.com>
 <1849610.1682807087@sss.pgh.pa.us>
 <bc6d1314-4e7a-2bde-1f01-a14de4909efd@gmail.com>
 <CAH2-Wzm3aQznsDrVu10C3hcvez8qwdy2578Hw4BgwXLWZ1NDQw@mail.gmail.com>
From: Alexander Lakhin <exclusion@gmail.com>
Content-Language: en-US
In-Reply-To: 
 <CAH2-Wzm3aQznsDrVu10C3hcvez8qwdy2578Hw4BgwXLWZ1NDQw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/88b5750f-a5da-798b-e5a1-af2c3ecb9b60%40gmail.com>
Precedence: bulk

Hello Peter,

30.04.2023 23:39, Peter Geoghegan wrote:
> On Sun, Apr 30, 2023 at 5:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
>> Yeah, also the environment changed since 86dc90056, so I couldn't just revert
>> that commit to test the ExecFilterJunk() behavior on master.
>>
>> All the questions you raised require a more thorough investigation.
> Are you aware of the fact that Valgrind has custom instrumentation
> that makes it directly capable of detecting access to no-longer-pinned
> buffers? See commits 7b7ed046 and 1e0dfd16.

Yes, I am. That's why I can't demonstrate real risks now — my repro
triggers a Valgrind warning only, and no other consequences are visible.

> I think that that may be a factor here. If it is, then it's a little
> surprising that you ever found the problem with ASAN, since of course
> we don't have custom ASAN instrumentation that tells ASAN things like
> "until I say otherwise, it is not okay to read from this range of
> memory, which is this backend's memory mapping for an individual
> shared buffer that was just unpinned".

The ASAN detected a consequence of the invalid memory read:
==1912964==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x7f223dd52c78 at pc 0x55c9e846146a bp 0x7fff0c63e560 sp 0x7fff0c63dd30
WRITE of size 42205122 at 0x7f223dd52c78 thread T0
...
#10 0x000055c9e84d09c5 in fill_val (...) at heaptuple.c:270
...
Note the abnormal size 42205122 (the target table contained much shorter
values at the moment). When exploring the coredump and the source code,
I found that that invalid size was gotten as follows:
ExecBRUpdateTriggers(): newtuple = ExecFetchSlotHeapTuple(newslot, true, &should_free_new);
ExecFetchSlotHeapTuple(): slot->tts_ops->materialize(slot)
tts_buffer_heap_materialize(): bslot->base.tuple = heap_form_tuple(...)
heap_form_tuple(): data_len = heap_compute_data_size(tupleDescriptor, values, isnull);
heap_compute_data_size(): here garbage data were read.

So I'm going to start it over and find a repro that will show something
interesting without Valgrind (and let estimate the probability of a
problem in the field).

Best regards,
Alexander