Re: BUG #17798: Incorrect memory access occurs when using BEFORE ROW UPDATE trigger

On Tue, Mar 7, 2023 at 2:00 PM Alexander Lakhin <exclusion(at)gmail(dot)com> wrote:

> > 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.

Reproduced this issue on master with your queries. I looked into this
issue and I agree with your analysis. I think this is exactly what
happened.

I also agree that we should materialize the newslot before we fetch
trigtuple from the oldslot which would materialize the oldslot and
release all buffer pins. But I'm not too familiar with the arounding
codes so need someone else to have a look.

Thanks
Richard