| From: | Amit Langote <amitlan(at)postgresql(dot)org> |
|---|---|
| To: | pgsql-committers(at)lists(dot)postgresql(dot)org |
| Subject: | pgsql: Don't free a cached RI plan that may be in use |
| Date: | 2026-11-04 06:46:43 |
| Message-ID: | E1xOUlf-00000000807-3NxE@gemulon.postgresql.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers |
Don't free a cached RI plan that may be in use
Currently, ri_FetchPreparedPlan() immediately frees a cached SPI plan
if SPI_plan_is_valid() returns false, without tracking whether the
plan might be in use. That can cause _SPI_execute_plan() to read
freed memory. This happens, for example, when an RI check's or
action's query runs some user-defined code such as a cast function or
BEFORE ROW trigger function which may perform its own DML using its
own query that is not restricted from firing its own triggers, some
of which may be RI triggers for the same constraint as the outer RI
check and hence use the same cached plan. If an invalidation arrives
that marks the cached plans referenced by the outer check's SPI plan
invalid between the outer check having called _SPI_execute_plan() and
the inner check's ri_FetchPreparedPlan(), the latter will free the
SPI plan as mentioned above.
Lucas Jeffrey reported an instance of this bug involving a
self-referencing ON DELETE CASCADE foreign key. The RI action's
DELETE fires a BEFORE DELETE trigger on the rows it deletes, which
invalidates the action's plan with GRANT and REVOKE on the table and
then runs a DELETE of its own, and the RI action fired by that DELETE
is the inner check described above. Separately, as Tom Lane
reported, buildfarm member trilobite hit the cast function case in
the regression tests, because CLOBBER_CACHE_ALWAYS makes the
invalidation certain. This trilobite failure came after 6aecabcd5ed
made the RI checks that require cast functions go through the SPI
path instead of the fast path, which didn't have this bug as it
didn't use SPI plans.
Fix by keeping a stack of the plans that every ri_PerformCheck() call
in progress is executing, pushed before SPI_execute_snapshot() and
popped whether it returns or throws an error, the latter using
PG_FINALLY(). ri_FetchPreparedPlan() now frees an invalid plan on the
spot when it is not on that stack, as before, since nothing can be
executing it. Otherwise it only detaches the plan from its hash entry
and queues it on a list. Whenever a check finishes, the queued plans
that are no longer on the stack are freed. So a plan is held only
while a check is actually executing it, however many invalidations
happen meanwhile. AtEOXact_RI() frees whatever is left, the same
treatment the fast-path metadata already gets since e73e0196.
Re-add the test case that f52d4051 removed because it turned trilobite
red, now that the bug is fixed, along with a comment noting that it
takes CLOBBER_CACHE_ALWAYS to hit the bug with it. Also, add the
test from Lucas's original patch, which reproduces the crash on an
ordinary cassert build, and a test that raises errors from inside a
check's query, some of them inside subtransactions that are then
rolled back, to check that the stack stays exact.
Author: Lucas Jeffrey <lucas(dot)jeffrey(at)anachronics(dot)com>
Co-authored-by: Trakshan Mishra <trakshanmishra477(at)gmail(dot)com>
Co-authored-by: Amit Langote <amitlangote09(at)gmail(dot)com>
Reported-by: Lucas Jeffrey <lucas(dot)jeffrey(at)anachronics(dot)com>
Reported-by: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Reviewed-by: Tomas Vondra <tomas(at)vondra(dot)me>
Discussion: https://postgr.es/m/3043944.1792163844@sss.pgh.pa.us
Discussion: https://postgr.es/m/CAGHzy7S0XfXdK3amd5c4QhpS1o9dySVzPr+-d9ohaE4iV_vNog@mail.gmail.com
Backpatch-through: 14
Branch
------
REL_19_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/fbe7033af494e1bcad0d6615d2f1f37576c9e8e3
Modified Files
--------------
src/backend/utils/adt/ri_triggers.c | 213 +++++++++++++++++++++++++++---
src/test/regress/expected/foreign_key.out | 186 ++++++++++++++++++++++++++
src/test/regress/sql/foreign_key.sql | 150 +++++++++++++++++++++
3 files changed, 530 insertions(+), 19 deletions(-)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Langote | 2026-11-04 06:46:57 | pgsql: Don't free a cached RI plan that may be in use |
| Previous Message | Amit Langote | 2026-11-04 06:46:29 | pgsql: Don't free a cached RI plan that may be in use |