From 6e8b80942bf9938b1e9bb59f613a4d072d6cea7e Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Fri, 7 Aug 2026 21:58:36 +0900 Subject: [PATCH v1 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. Discussion: https://postgr.es/m/CA+HiwqFFB6vzx8v3t2=rbNYyxMistLf5kkJfqzJ81nadFyLrxA@mail.gmail.com Backpatch-through: 19 --- src/backend/utils/adt/ri_triggers.c | 47 ++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 6f201ef0f31..bce3b6775ee 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -159,6 +159,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; /* @@ -271,6 +274,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 */ @@ -2560,6 +2571,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, @@ -2593,11 +2608,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); } @@ -3551,8 +3580,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]; @@ -4366,6 +4397,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); + } } /* -- 2.47.3