Content-Type: multipart/mixed; boundary="------------t0xaa0vFwpI0asFivMd0YgTP"
Message-ID: <87fe6c28-80b2-f709-a8db-d105e5240e39@gmail.com>
Date: Tue, 7 Mar 2023 09:00:01 +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
Content-Language: en-US
To: pgsql-bugs@lists.postgresql.org
References: <17798-0907404928dcf0dd@postgresql.org>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <17798-0907404928dcf0dd@postgresql.org>
Archived-At: 
 <https://www.postgresql.org/message-id/87fe6c28-80b2-f709-a8db-d105e5240e39%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------t0xaa0vFwpI0asFivMd0YgTP
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

17.02.2023 13:00, PG Bug reporting form wrote:
> The following bug has been logged on the website:
> ...
>
> I've found the following explanation for the failure:
> 1) After the ExecGetUpdateNewTuple() call the newslot and the oldslot are
>   linked together (their slot->tts_values[1] in this case point to the same
>   memory address (inside the oldslot' buffer)).
> 2) Previously, GetTupleForTrigger() could get a tuple with a new buffer,
>   so the oldslot would be the only user of the buffer at that moment.
>   (The newslot doesn't become an official user of the buffer.)
> 3) Then, trigtuple = ExecFetchSlotHeapTuple(oldslot, ...) invokes
>   tts_buffer_heap_materialize() where the oldslot->buffer is released.
> 4) Finally, newtuple = ExecFetchSlotHeapTuple(newslot, ...) invokes
>   tts_buffer_heap_materialize() where an incorrect access to memory
>   that became anonymous occurs, and that is detected by valgrind.
>   If not detected, different consequences are possible (in the asan case
>   it was memcpy with an incorrectly read extra large data_len).
I've tried to materialize newslot before the oldslot materialization
(in ExecFetchSlotHeapTuple(), where their common memory is released),
and it looks like it fixes the issue.
The similar thing done in 75e03eabe.
But I don't know the code good enough so maybe I'm missing something.

Best regards,
Alexander
--------------t0xaa0vFwpI0asFivMd0YgTP
Content-Type: text/x-patch; charset=UTF-8;
 name="v1-01-Fix-memory-access-in-BRU-trigger-DRAFT.patch"
Content-Disposition: attachment;
 filename="v1-01-Fix-memory-access-in-BRU-trigger-DRAFT.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL2NvbW1hbmRzL3RyaWdnZXIuYyBiL3NyYy9iYWNr
ZW5kL2NvbW1hbmRzL3RyaWdnZXIuYwppbmRleCA2NjQwMWYyODM5Li5hNDA5ZmYwYmUxIDEw
MDY0NAotLS0gYS9zcmMvYmFja2VuZC9jb21tYW5kcy90cmlnZ2VyLmMKKysrIGIvc3JjL2Jh
Y2tlbmQvY29tbWFuZHMvdHJpZ2dlci5jCkBAIC0yOTkyLDYgKzI5OTIsOCBAQCBFeGVjQlJV
cGRhdGVUcmlnZ2VycyhFU3RhdGUgKmVzdGF0ZSwgRVBRU3RhdGUgKmVwcXN0YXRlLAogCiAJ
CQlpZiAobmV3c2xvdCAhPSBlcHFzbG90X2NsZWFuKQogCQkJCUV4ZWNDb3B5U2xvdChuZXdz
bG90LCBlcHFzbG90X2NsZWFuKTsKKwkJCWVsc2UKKwkJCQlFeGVjTWF0ZXJpYWxpemVTbG90
KG5ld3Nsb3QpOwogCQl9CiAKIAkJdHJpZ3R1cGxlID0gRXhlY0ZldGNoU2xvdEhlYXBUdXBs
ZShvbGRzbG90LCB0cnVlLCAmc2hvdWxkX2ZyZWVfdHJpZyk7Cg==

--------------t0xaa0vFwpI0asFivMd0YgTP--