| 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:45:31 |
| Message-ID: | E1xOUkV-000000007t2-4AIF@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, Tom Lane reported
that buildfarm member trilobite, which runs with CLOBBER_CACHE_ALWAYS,
hit this in the regression tests on master after a change there made
more FK checks go through SPI.
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. A new AtEOXact_RI() frees whatever is left.
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_14_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/bc2125742cb0ccacad71d4883da9369669846017
Modified Files
--------------
src/backend/access/transam/xact.c | 3 +
src/backend/utils/adt/ri_triggers.c | 207 ++++++++++++++++++++++++++++--
src/include/commands/trigger.h | 1 +
src/test/regress/expected/foreign_key.out | 156 ++++++++++++++++++++++
src/test/regress/sql/foreign_key.sql | 125 ++++++++++++++++++
5 files changed, 484 insertions(+), 8 deletions(-)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Langote | 2026-11-04 06:45:46 | pgsql: Don't free a cached RI plan that may be in use |
| Previous Message | Nikolay Samokhvalov | 2026-11-04 06:41:15 | Re: pgsql: Reduce "Var IS [NOT] NULL" quals during constant folding |