Re: Use-after-free in reorderbuffer.c for INSERT ON CONFLICT

On Fri, Aug 1, 2025 at 10:22 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Fri, Aug 01, 2025 at 10:03:14AM +0530, Amit Kapila wrote:
> > We still won't be able to capture the latest LSN in case of
> > REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT. IIRC, update_progress_txn
> > is used to keep the client active so that when many changes are
> > skipped, the client doesn't timeout. In this case, it seems okay to
> > use prev_lsn as well.
>
> I am not quite sure to follow your argument here. In the case of a
> REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT change, we would use
> change->lsn, which is in the case of the patch and HEAD the same
> thing: prev_lsn.
>

I mean to say we can use the same change LSN both for
REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM and
REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT. Right now, for
REORDER_BUFFER_CHANGE_INTERNAL_SPEC_CONFIRM, we switch the change to
specinsert which would have a prior LSN value (say, if confirm/abort
record will have value, 1000, it will be 800 or so) but we should
still use 1000 for update_progress_txn. The update_progress_txn() is
helpful when such an insert is skipped by a plugin (in this case
pgouput) and in that case, we would require the latest LSN processed
by reorder buffer to pass to it. We use it to send a keep_alive to a
client with the last LSN processed.

--
With Regards,
Amit Kapila.