From 6251c1c0a5ff21667318d198bdf89cd4889c3a0d Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Tue, 18 Aug 2026 21:34:24 +0900
Subject: [PATCH v2 1/2] Don't free fast-path FK metadata from the inval
 callback

Commit e484b0eea6 made InvalidateConstraintCacheCallBack() pfree an
entry's FastPathMeta to plug a leak, but that breaks the rule stated
atop the callback: entries may have active references at invalidation
time, so we mark them invalid rather than removing them.

The metadata is subject to the same rule.  ri_FastPathCheck() and
ri_FastPathBatchFlush() copy riinfo->fpmeta into a local, and
ri_FastPathFlushArray() additionally takes FmgrInfo pointers into it
before its "walk all matches" loop.  That loop runs
index_getnext_slot(), ri_LockPKTuple(), and user-supplied cast and
equality functions, any of which can accept invalidation messages, and
a user function that performs DDL triggers one deliberately, no
concurrency required.  The callback then freed the object still in use,
so the loop read freed memory and called through FmgrInfos in it.

Fix by unlinking the metadata from the entry, so the next check
rebuilds it as before, but deferring the actual free to AtEOXact_RI(),
which runs from CommitTransaction() / PrepareTransaction() /
AbortTransaction() with no RI check on the stack.  Detached objects
are chained through a new next_dead field and released there.

The queue holds a few kB per detached object until the transaction
ends, which only affects transactions that interleave DDL with
FK-checking DML. That seems clearly preferable to a use-after-free.

Unlinking alone is not enough for the multi-column path.
ri_FastPathFlushLoop() calls build_index_scankeys() once per buffered
row, and that function re-read riinfo->fpmeta each time.  A cast
invoked for one row can accept an invalidation that clears the field,
so the next row found NULL there.  Pass the metadata down from
ri_FastPathBatchFlush() instead, as the array path already did, so one
reference covers the whole batch.  That is only safe because of the
deferred release above; latching without it would turn the NULL
dereference into a use-after-free.

Reported-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqFFB6vzx8v3t2=rbNYyxMistLf5kkJfqzJ81nadFyLrxA@mail.gmail.com
Backpatch-through: 19
---
 src/backend/utils/adt/ri_triggers.c       | 89 +++++++++++++++++++----
 src/test/regress/expected/foreign_key.out | 64 ++++++++++++++++
 src/test/regress/sql/foreign_key.sql      | 56 ++++++++++++++
 3 files changed, 195 insertions(+), 14 deletions(-)

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 5d843ce8a0d..5be183706e8 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -160,6 +160,9 @@ typedef struct FastPathMeta
 	Oid			subtypes[RI_MAX_NUMKEYS];
 	int			strats[RI_MAX_NUMKEYS];
 	AttrNumber	index_attnos[RI_MAX_NUMKEYS];	/* index column positions */
+
+	/* Link in ri_fpmeta_dead_list while awaiting deferred release */
+	struct FastPathMeta *next_dead;
 } FastPathMeta;
 
 /*
@@ -272,6 +275,14 @@ static HTAB *ri_fastpath_cache = NULL;
 static bool ri_fastpath_callback_registered = false;
 static bool ri_fastpath_flushing = false;
 
+/*
+ * FastPathMeta objects detached from their cache entry by invalidation, but
+ * possibly still referenced by an RI check further up the stack.  Released
+ * by AtEOXact_RI(), where no such reference can exist.  See
+ * InvalidateConstraintCacheCallBack().
+ */
+static FastPathMeta *ri_fpmeta_dead_list = NULL;
+
 /*
  * Local function prototypes
  */
@@ -326,10 +337,12 @@ static void ri_FastPathBatchAdd(RI_ConstraintInfo *riinfo,
 static void ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel,
 								  RI_ConstraintInfo *riinfo);
 static int	ri_FastPathFlushArray(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot,
-								  const RI_ConstraintInfo *riinfo, Relation fk_rel,
+								  const RI_ConstraintInfo *riinfo,
+								  FastPathMeta *fpmeta, Relation fk_rel,
 								  Snapshot snapshot, IndexScanDesc scandesc);
 static int	ri_FastPathFlushLoop(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot,
-								 const RI_ConstraintInfo *riinfo, Relation fk_rel,
+								 const RI_ConstraintInfo *riinfo,
+								 FastPathMeta *fpmeta, Relation fk_rel,
 								 Snapshot snapshot, IndexScanDesc scandesc);
 static bool ri_FastPathProbeOne(Relation pk_rel, Relation idx_rel,
 								IndexScanDesc scandesc, TupleTableSlot *slot,
@@ -342,6 +355,7 @@ static void ri_CheckPermissions(Relation query_rel);
 static bool recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys,
 									 int nkeys, TupleTableSlot *new_slot);
 static void build_index_scankeys(const RI_ConstraintInfo *riinfo,
+								 FastPathMeta *fpmeta,
 								 Relation idx_rel, Datum *pk_vals,
 								 char *pk_nulls, ScanKey skeys);
 static void ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo,
@@ -2561,6 +2575,10 @@ get_ri_constraint_root(Oid constrOid)
  * from the cache, but only mark them invalid, which is harmless to active
  * uses.  (Any query using an entry should hold a lock sufficient to keep that
  * data from changing under it --- but we may get cache flushes anyway.)
+ *
+ * The fast-path metadata hanging off an entry is subject to the same rule.
+ * We unlink it so that the next check rebuilds it, but the object itself is
+ * only queued here and is actually released by AtEOXact_RI().
  */
 static void
 InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
@@ -2594,11 +2612,25 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
 			riinfo->rootHashValue == hashvalue)
 		{
 			riinfo->valid = false;
+
+			/*
+			 * Detach any fast-path metadata so that the next check
+			 * repopulates it, but do not free it here.  ri_FastPathCheck()
+			 * and the flush routines copy riinfo->fpmeta into a local (and
+			 * take FmgrInfo pointers into it) and then run index scans, tuple
+			 * locking, and user-supplied cast and equality functions, all of
+			 * which can accept invalidation messages and reach this callback.
+			 * Freeing now would leave those callers reading freed memory.
+			 * Queue it instead; AtEOXact_RI() releases it once no RI check
+			 * can be running.
+			 */
 			if (riinfo->fpmeta)
 			{
-				pfree(riinfo->fpmeta);
+				riinfo->fpmeta->next_dead = ri_fpmeta_dead_list;
+				ri_fpmeta_dead_list = riinfo->fpmeta;
 				riinfo->fpmeta = NULL;
 			}
+
 			/* Remove invalidated entries from the list, too */
 			dclist_delete_from(&ri_constraint_cache_valid_list, iter.cur);
 		}
@@ -2862,7 +2894,8 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo,
 	}
 	Assert(riinfo->fpmeta);
 	ri_ExtractValues(fk_rel, newslot, riinfo, false, pk_vals, pk_nulls);
-	build_index_scankeys(riinfo, idx_rel, pk_vals, pk_nulls, skey);
+	build_index_scankeys(riinfo, riinfo->fpmeta, idx_rel, pk_vals, pk_nulls,
+						 skey);
 	found = ri_FastPathProbeOne(pk_rel, idx_rel, scandesc, slot,
 								snapshot, riinfo, skey, riinfo->nkeys);
 	SetUserIdAndSecContext(saved_userid, saved_sec_context);
@@ -2954,6 +2987,7 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel,
 	Oid			saved_userid;
 	int			saved_sec_context;
 	MemoryContext oldcxt;
+	FastPathMeta *fpmeta;
 	int			violation_index;
 
 	if (fpentry->batch_count == 0)
@@ -3011,6 +3045,15 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel,
 	}
 	Assert(riinfo->fpmeta);
 
+	/*
+	 * Take our own reference to the metadata for the duration of the flush.
+	 * The probe below runs user-defined cast and equality functions, which can
+	 * accept invalidation messages; InvalidateConstraintCacheCallBack() then
+	 * clears riinfo->fpmeta, so re-reading it partway through the batch would
+	 * find NULL.  The object itself stays valid until AtEOXact_RI().
+	 */
+	fpmeta = riinfo->fpmeta;
+
 	/*
 	 * The probe runs user-defined cast and equality functions.  Set the
 	 * flushing flag around it so a re-entrant ri_FastPathBatchAdd on this
@@ -3024,10 +3067,12 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel,
 		/* Skip array overhead for single-row batches. */
 		if (riinfo->nkeys == 1 && fpentry->batch_count > 1)
 			violation_index = ri_FastPathFlushArray(fpentry, fk_slot, riinfo,
-													fk_rel, snapshot, scandesc);
+													fpmeta, fk_rel, snapshot,
+													scandesc);
 		else
 			violation_index = ri_FastPathFlushLoop(fpentry, fk_slot, riinfo,
-												   fk_rel, snapshot, scandesc);
+												   fpmeta, fk_rel, snapshot,
+												   scandesc);
 	}
 	PG_FINALLY();
 	{
@@ -3065,8 +3110,9 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel,
  */
 static int
 ri_FastPathFlushLoop(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot,
-					 const RI_ConstraintInfo *riinfo, Relation fk_rel,
-					 Snapshot snapshot, IndexScanDesc scandesc)
+					 const RI_ConstraintInfo *riinfo, FastPathMeta *fpmeta,
+					 Relation fk_rel, Snapshot snapshot,
+					 IndexScanDesc scandesc)
 {
 	Relation	pk_rel = fpentry->pk_rel;
 	Relation	idx_rel = fpentry->idx_rel;
@@ -3080,7 +3126,7 @@ ri_FastPathFlushLoop(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot,
 	{
 		ExecStoreHeapTuple(fpentry->batch[i], fk_slot, false);
 		ri_ExtractValues(fk_rel, fk_slot, riinfo, false, pk_vals, pk_nulls);
-		build_index_scankeys(riinfo, idx_rel, pk_vals, pk_nulls, skey);
+		build_index_scankeys(riinfo, fpmeta, idx_rel, pk_vals, pk_nulls, skey);
 
 		found = ri_FastPathProbeOne(pk_rel, idx_rel, scandesc, pk_slot,
 									snapshot, riinfo, skey, riinfo->nkeys);
@@ -3109,10 +3155,10 @@ ri_FastPathFlushLoop(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot,
  */
 static int
 ri_FastPathFlushArray(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot,
-					  const RI_ConstraintInfo *riinfo, Relation fk_rel,
-					  Snapshot snapshot, IndexScanDesc scandesc)
+					  const RI_ConstraintInfo *riinfo, FastPathMeta *fpmeta,
+					  Relation fk_rel, Snapshot snapshot,
+					  IndexScanDesc scandesc)
 {
-	FastPathMeta *fpmeta = riinfo->fpmeta;
 	Relation	pk_rel = fpentry->pk_rel;
 	Relation	idx_rel = fpentry->idx_rel;
 	TupleTableSlot *pk_slot = fpentry->pk_slot;
@@ -3496,11 +3542,10 @@ recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys, int nkeys,
  */
 static void
 build_index_scankeys(const RI_ConstraintInfo *riinfo,
+					 FastPathMeta *fpmeta,
 					 Relation idx_rel, Datum *pk_vals,
 					 char *pk_nulls, ScanKey skeys)
 {
-	FastPathMeta *fpmeta = riinfo->fpmeta;
-
 	Assert(fpmeta);
 
 	/*
@@ -3553,8 +3598,10 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo,
 	MemoryContext oldcxt = MemoryContextSwitchTo(TopMemoryContext);
 
 	Assert(riinfo != NULL && riinfo->valid);
+	Assert(riinfo->fpmeta == NULL);
 
 	fpmeta = palloc_object(FastPathMeta);
+	fpmeta->next_dead = NULL;
 	for (int i = 0; i < riinfo->nkeys; i++)
 	{
 		Oid			eq_opr = riinfo->pf_eq_oprs[i];
@@ -4368,6 +4415,20 @@ AtEOXact_RI(bool isCommit)
 	 * set.
 	 */
 	ri_fastpath_flushing = false;
+
+	/*
+	 * Release fast-path metadata detached during this transaction by
+	 * InvalidateConstraintCacheCallBack().  We are past every RI check that
+	 * could still hold a pointer into one of these, so freeing here is safe
+	 * on both the commit and the abort path.
+	 */
+	while (ri_fpmeta_dead_list != NULL)
+	{
+		FastPathMeta *dead = ri_fpmeta_dead_list;
+
+		ri_fpmeta_dead_list = dead->next_dead;
+		pfree(dead);
+	}
 }
 
 /*
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 120d3319451..b699164260a 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -3846,3 +3846,67 @@ DETAIL:  Key (a)=(999) is not present in table "fp_subxact_pk".
 DROP TRIGGER fp_subxact_trg ON fp_subxact_fk;
 DROP FUNCTION fp_abort_subxact();
 DROP TABLE fp_subxact_fk, fp_subxact_pk;
+--
+-- Cache invalidation arriving in the middle of a fast-path batch flush.
+--
+-- A cross-type foreign key runs the user's cast function once per key per
+-- buffered row, inside ri_FastPathFlushLoop().  A cast that performs DDL
+-- raises an invalidation there, which detaches the constraint's fast-path
+-- metadata while the flush is still using it.
+--
+CREATE TYPE fkint;
+CREATE FUNCTION fkint_in(cstring) RETURNS fkint
+  AS 'int4in' LANGUAGE internal IMMUTABLE STRICT;
+NOTICE:  return type fkint is only a shell
+CREATE FUNCTION fkint_out(fkint) RETURNS cstring
+  AS 'int4out' LANGUAGE internal IMMUTABLE STRICT;
+NOTICE:  argument type fkint is only a shell
+LINE 1: CREATE FUNCTION fkint_out(fkint) RETURNS cstring
+                                  ^
+CREATE TYPE fkint (INPUT = fkint_in, OUTPUT = fkint_out, LIKE = int4);
+-- Renames the constraint the first time it is called, and so raises an
+-- invalidation partway through the flush.  Guarded on the catalog so the
+-- second and later calls are no-ops.
+CREATE FUNCTION fkint_to_int4(fkint) RETURNS int4 AS $$
+BEGIN
+    IF EXISTS (SELECT 1 FROM pg_constraint
+                WHERE conrelid = 'fktable_inval'::regclass
+                  AND conname = 'fktable_inval_fk') THEN
+        EXECUTE 'ALTER TABLE fktable_inval'
+                ' RENAME CONSTRAINT fktable_inval_fk TO fktable_inval_fk2';
+    END IF;
+    RETURN format('%s', $1)::int4;
+END $$ LANGUAGE plpgsql;
+CREATE CAST (fkint AS int4) WITH FUNCTION fkint_to_int4(fkint) AS IMPLICIT;
+CREATE TABLE pktable_inval (a int4, b int4, PRIMARY KEY (a, b));
+INSERT INTO pktable_inval VALUES (1, 1), (2, 2);
+-- Multi-column FK, so the flush takes the per-row loop rather than the
+-- array path; cross-type on column a, so the cast above is invoked.
+CREATE TABLE fktable_inval (a fkint, b int4,
+  CONSTRAINT fktable_inval_fk FOREIGN KEY (a, b)
+    REFERENCES pktable_inval (a, b));
+-- More than one row, so the flush is still running after the invalidation.
+INSERT INTO fktable_inval VALUES ('1', 1), ('2', 2);
+-- Confirms the cast actually ran and raised the invalidation.  Without this
+-- the insert above could pass merely by not exercising the path at all.
+SELECT conname FROM pg_constraint
+  WHERE conrelid = 'fktable_inval'::regclass AND contype = 'f';
+      conname      
+-------------------
+ fktable_inval_fk2
+(1 row)
+
+SELECT count(*) FROM fktable_inval;
+ count 
+-------
+     2
+(1 row)
+
+DROP TABLE fktable_inval;
+DROP TABLE pktable_inval;
+DROP CAST (fkint AS int4);
+DROP FUNCTION fkint_to_int4(fkint);
+DROP TYPE fkint CASCADE;
+NOTICE:  drop cascades to 2 other objects
+DETAIL:  drop cascades to function fkint_in(cstring)
+drop cascades to function fkint_out(fkint)
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index b9b88064ea5..31736251d78 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -2801,3 +2801,59 @@ INSERT INTO fp_subxact_fk VALUES (999, 'bad'), (0, 'boom'), (1, 'ok');
 DROP TRIGGER fp_subxact_trg ON fp_subxact_fk;
 DROP FUNCTION fp_abort_subxact();
 DROP TABLE fp_subxact_fk, fp_subxact_pk;
+
+--
+-- Cache invalidation arriving in the middle of a fast-path batch flush.
+--
+-- A cross-type foreign key runs the user's cast function once per key per
+-- buffered row, inside ri_FastPathFlushLoop().  A cast that performs DDL
+-- raises an invalidation there, which detaches the constraint's fast-path
+-- metadata while the flush is still using it.
+--
+CREATE TYPE fkint;
+CREATE FUNCTION fkint_in(cstring) RETURNS fkint
+  AS 'int4in' LANGUAGE internal IMMUTABLE STRICT;
+CREATE FUNCTION fkint_out(fkint) RETURNS cstring
+  AS 'int4out' LANGUAGE internal IMMUTABLE STRICT;
+CREATE TYPE fkint (INPUT = fkint_in, OUTPUT = fkint_out, LIKE = int4);
+
+-- Renames the constraint the first time it is called, and so raises an
+-- invalidation partway through the flush.  Guarded on the catalog so the
+-- second and later calls are no-ops.
+CREATE FUNCTION fkint_to_int4(fkint) RETURNS int4 AS $$
+BEGIN
+    IF EXISTS (SELECT 1 FROM pg_constraint
+                WHERE conrelid = 'fktable_inval'::regclass
+                  AND conname = 'fktable_inval_fk') THEN
+        EXECUTE 'ALTER TABLE fktable_inval'
+                ' RENAME CONSTRAINT fktable_inval_fk TO fktable_inval_fk2';
+    END IF;
+    RETURN format('%s', $1)::int4;
+END $$ LANGUAGE plpgsql;
+
+CREATE CAST (fkint AS int4) WITH FUNCTION fkint_to_int4(fkint) AS IMPLICIT;
+
+CREATE TABLE pktable_inval (a int4, b int4, PRIMARY KEY (a, b));
+INSERT INTO pktable_inval VALUES (1, 1), (2, 2);
+
+-- Multi-column FK, so the flush takes the per-row loop rather than the
+-- array path; cross-type on column a, so the cast above is invoked.
+CREATE TABLE fktable_inval (a fkint, b int4,
+  CONSTRAINT fktable_inval_fk FOREIGN KEY (a, b)
+    REFERENCES pktable_inval (a, b));
+
+-- More than one row, so the flush is still running after the invalidation.
+INSERT INTO fktable_inval VALUES ('1', 1), ('2', 2);
+
+-- Confirms the cast actually ran and raised the invalidation.  Without this
+-- the insert above could pass merely by not exercising the path at all.
+SELECT conname FROM pg_constraint
+  WHERE conrelid = 'fktable_inval'::regclass AND contype = 'f';
+
+SELECT count(*) FROM fktable_inval;
+
+DROP TABLE fktable_inval;
+DROP TABLE pktable_inval;
+DROP CAST (fkint AS int4);
+DROP FUNCTION fkint_to_int4(fkint);
+DROP TYPE fkint CASCADE;
-- 
2.47.3

