| From: | Amit Langote <amitlan(at)postgresql(dot)org> |
|---|---|
| To: | pgsql-committers(at)lists(dot)postgresql(dot)org |
| Subject: | pgsql: Don't free fast-path FK metadata from the inval callback |
| Date: | 2026-08-19 07:09:31 |
| Message-ID: | E1wwaQU-00000001B27-3UAz@gemulon.postgresql.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers |
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: Jacob Brazeal <jacob(dot)brazeal(at)gmail(dot)com> (offlist)
Reported-by: Brian Carpenter | Deep Fork Cyber <b(at)deepforkcyber(dot)com>
<offlist>
Reported-by: Anh Khoa <blkhoa2004(at)gmail(dot)com> (offlist)
Reported-by: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
Reviewed-by: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
Discussion: https://postgr.es/m/CA+HiwqFFB6vzx8v3t2=rbNYyxMistLf5kkJfqzJ81nadFyLrxA@mail.gmail.com
Discussion: https://postgr.es/m/CAJTYsWUFBZNs_UN5MAAK7vG4_DEmv0FiT8552CWuOcggDUki2g@mail.gmail.com
Backpatch-through: 19
Branch
------
REL_19_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/18a15b9673809c6b4848575aa14aaa8e0e5c7a33
Modified Files
--------------
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(-)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Langote | 2026-08-19 07:09:46 | pgsql: Don't free fast-path FK metadata from the inval callback |
| Previous Message | Peter Eisentraut | 2026-08-19 07:02:28 | pgsql: Message wording fix |