From: Michael Paquier <michael@paquier.xyz>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Postgres bugs <pgsql-bugs@lists.postgresql.org>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Ethan Mertz <ethan.mertz@gmail.com>
Subject: Re: Use-after-free in reorderbuffer.c for INSERT ON CONFLICT
Date: Fri, 1 Aug 2025 20:45:10 +0900
Message-ID: <aIyoxicMnn2ebKLa@paquier.xyz> (raw)
In-Reply-To: <CAA4eK1L3T+9Scoi=g=0QjaJP6pLUZh8iDMMY30cgA4GbY_Y_AA@mail.gmail.com>
References: <aIsQqDZ7x4LAQ6u1@paquier.xyz>
<CAA4eK1L8x5CgzShZ2FLW0QLUVqQZxjp9MLpeFbHBqUeN++VEDA@mail.gmail.com>
<aIxIBL61XtPxXJVV@paquier.xyz>
<CAA4eK1L3T+9Scoi=g=0QjaJP6pLUZh8iDMMY30cgA4GbY_Y_AA@mail.gmail.com>
On Fri, Aug 01, 2025 at 03:30:17PM +0530, Amit Kapila wrote:
> 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.
Ah, OK, I've missed your point then. It's kind of an optimization in
itself because we would be a bit more aggressive with the updates, but
I agree to do that in the scope of this fix. The updated attached
uses prev_lsn for the job, for both the ABORT and CONFIRM cases,
meaning a one-liner.
--
Michael
Attachments:
[text/x-diff] v2-0001-Fix-use-after-free-in-reorderbuffer.c-with-ON-CON.patch (892B, ../aIyoxicMnn2ebKLa@paquier.xyz/2-v2-0001-Fix-use-after-free-in-reorderbuffer.c-with-ON-CON.patch)
download | inline diff:
From df18f6e51050193c739397056c80f82f3002dbb6 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Fri, 1 Aug 2025 20:35:05 +0900
Subject: [PATCH v2] Fix use-after-free in reorderbuffer.c with ON CONFLICT
changes
---
src/backend/replication/logical/reorderbuffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.cindex 5febd154b6ba..34cf05668ae8 100644--- a/src/backend/replication/logical/reorderbuffer.c+++ b/src/backend/replication/logical/reorderbuffer.c@@ -2599,7 +2599,7 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
if (++changes_count >= CHANGES_THRESHOLD)
{
- rb->update_progress_txn(rb, txn, change->lsn);+ rb->update_progress_txn(rb, txn, prev_lsn);
changes_count = 0;
}
}
--
2.50.0
[application/pgp-signature] signature.asc (832B, ../aIyoxicMnn2ebKLa@paquier.xyz/3-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, amit.kapila16@gmail.com, pgsql-bugs@lists.postgresql.org, sawada.mshk@gmail.com, ethan.mertz@gmail.com
Subject: Re: Use-after-free in reorderbuffer.c for INSERT ON CONFLICT
In-Reply-To: <aIyoxicMnn2ebKLa@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox