Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 1 Aug 2025 20:45:10 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Postgres bugs <pgsql-bugs@lists.postgresql.org>,
	Masahiko Sawada <sawada.mshk@gmail.com>,
	Ethan Mertz <ethan.mertz@gmail.com>
Subject: Re: Use-after-free in reorderbuffer.c for INSERT ON CONFLICT
Message-ID: <aIyoxicMnn2ebKLa@paquier.xyz>
References: <aIsQqDZ7x4LAQ6u1@paquier.xyz>
 <CAA4eK1L8x5CgzShZ2FLW0QLUVqQZxjp9MLpeFbHBqUeN++VEDA@mail.gmail.com>
 <aIxIBL61XtPxXJVV@paquier.xyz>
 <CAA4eK1L3T+9Scoi=g=0QjaJP6pLUZh8iDMMY30cgA4GbY_Y_AA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="qeLOSZoHqZHPEEvu"
Content-Disposition: inline
In-Reply-To: 
 <CAA4eK1L3T+9Scoi=g=0QjaJP6pLUZh8iDMMY30cgA4GbY_Y_AA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aIyoxicMnn2ebKLa%40paquier.xyz>
Precedence: bulk


--qeLOSZoHqZHPEEvu
Content-Type: multipart/mixed; boundary="f7jaUnqZwvisdiDh"
Content-Disposition: inline


--f7jaUnqZwvisdiDh
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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

--f7jaUnqZwvisdiDh
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0001-Fix-use-after-free-in-reorderbuffer.c-with-ON-CON.patch"
Content-Transfer-Encoding: quoted-printable

=46rom df18f6e51050193c739397056c80f82f3002dbb6 Mon Sep 17 00:00:00 2001
=46rom: 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.c
index 5febd154b6ba..34cf05668ae8 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2599,7 +2599,7 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBuf=
ferTXN *txn,
=20
 			if (++changes_count >=3D CHANGES_THRESHOLD)
 			{
-				rb->update_progress_txn(rb, txn, change->lsn);
+				rb->update_progress_txn(rb, txn, prev_lsn);
 				changes_count =3D 0;
 			}
 		}
--=20
2.50.0


--f7jaUnqZwvisdiDh--

--qeLOSZoHqZHPEEvu
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmiMqMYACgkQnvQgOdby
QH1iAQ//fl/EU76+Md7SHvQB98v+AzRJPpEexrkNNq9CKBPAXOy33LyocWPIMd9v
feVORbrIDABhJb15W0Tyh0BYi9aS+H+BcJqx2osFZaEgWkmkmicxOYG2bK7UQiF3
srtTpJVhvQBtZZPI0T/raSsN+VTWrbakSyx0zkWUtHVoV+zvASgVCQOk7B3jXTGs
JJM9+03OpanHD9lXKnf88alxY+zycG5qby7wo5FDbFnkY3qM839mre7izBQ0AmiM
jRNc+Q5QrNRmAid2esroGNHJRvmbt/6meYgPhE06PAAHL1a1KtPxoxszpbB0q+X7
GASN8GuwXdCl8LbpgBWiQXvuF/J10h7s9nXajCdDsh0Ky2sMnGLsMjsF4Ug2R9iB
hbHHG+cbalyKCqvXDf7m6U1MCDAfWTeDSKsjWnpeIo3PkkNP7Ym9UAqvqjIFDfq+
IYr2oTjEV4v7ocAdi1Bllt21U61M8uOEeUpkcqqkEtDhRxdsOM126tSb5nx056Sp
1yc65EsYWFIen06oGt4K35S7oj9CxbtKgSmLqEbEUt7mUXKHbHFUWjknuHzDexSC
pU0fvoNUn6yi9LTPtP0TZOJreKTuXcvv21Y9dWUzFSUv2ehPzkJfxWRVnU+7pVry
zTY6NA2ecTduH0+OwhSkdjq1076IyVgLSLC1jkWjIh9lE22BAok=
=PTcj
-----END PGP SIGNATURE-----

--qeLOSZoHqZHPEEvu--