Re: [PATCH] Fix segmentation fault caused by reentrancy in RI_Fkey_cascade_del (ri_triggers.c)

Hi,
Gentle reminder, is there anyone reviewing this?
I can give some help to review this patch if needed, or help out reviewing
another patch in exchange, just let me know please.
Regards
- Lucas J.

El vie, 14 ago 2026 a las 20:22, Amit Langote (<amitlangote09(at)gmail(dot)com>)
escribió:

> Hi Alvaro,
>
> On Fri, Aug 14, 2026 at 21:37 Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
>> [offlist] Hi Amit,
>>
>> Lucas reported this crash in the RI code two months ago. I have not had
>> a chance to review it and probably won't yet for another month, so if
>> you have a moment, I think you may have the code fresher in your head to
>> better assess whether his proposed fix is the most appropriate one. If
>> you don't, no problem, I will probably spend some time on this
>> eventually. We may not want to leave such a trivially invoked crasher
>> in the tree for much longer -- it's probably security-relevant.
>>
>>
>> https://postgr.es/m/CAGHzy7S0XfXdK3amd5c4QhpS1o9dySVzPr+-d9ohaE4iV_vNog@mail.gmail.com
>
>
> Ok, I will take a look next week after I’ve finished dealing with all the
> reported RI fastpath issues.
>
> - Amit
>
> On 2026-Jun-01, Lucas Jeffrey wrote:
>>
>> > Hi hackers
>> > I detected some failing tests and I created a new version of the patch
>> that
>> > fixes those errors in regression tests.
>> >
>> > El vie, 29 may 2026 a las 12:32, Lucas Jeffrey (<
>> > lucas(dot)jeffrey(at)anachronics(dot)com>) escribió:
>> >
>> > > Hi hackers,
>> > >
>> > > We found a bug where executing a DELETE on a self-referential table
>> that
>> > > fires triggers can cause a segmentation fault. This is due to a
>> > > *use-after-free* of a Postgres plan generated by the referential
>> > > integrity module (ri_triggers.c, RI_FKey_cascade_del). The crash
>> occurs
>> > > if the Postgres plancache is invalidated (ResetPlanCache) during the
>> > > execution of a reentrant RI trigger.
>> > >
>> > > A reentrant RI_FKey_cascade_del can occur if a table is
>> self-referential
>> > > (i.e., it has a foreign key referencing its own primary key) and has
>> BEFORE
>> > > DELETE triggers that delete rows from that same table.
>> > >
>> > > -
>> > >
>> > > *The first patch* adds a test case that reproduces the segmentation
>> > > fault. The crash itself happens in _SPI_execute_plan, but the root
>> > > cause is that the plan being executed was prematurely freed by the
>> RI
>> > > module.
>> > > -
>> > >
>> > > *The second patch* fixes ri_triggers.c by introducing reentrancy
>> > > guards, which maintain a reference count of plans in execution to
>> prevent
>> > > them from being freed while active.
>> > >
>> > > Feedback and reviews are welcome.
>> > >
>> > > Best regards,
>> > >
>> > > Lucas Jeffrey
>> > >
>>
>> > From 534002b0999c6e620c055f3e52634937c4693849 Mon Sep 17 00:00:00 2001
>> > From: luquijeffrey <lucas(dot)jeffrey(at)anachronics(dot)com>
>> > Date: Fri, 29 May 2026 12:23:38 -0300
>> > Subject: [PATCH 1/2]
>> =?UTF-8?q?Add=20test=20case=20that=20triggers=20self?=
>> > =?UTF-8?q?=E2=80=91referencing=20table=20crash=20(patch1)?=
>> > MIME-Version: 1.0
>> > Content-Type: text/plain; charset=UTF-8
>> > Content-Transfer-Encoding: 8bit
>> >
>> > ---
>> > .../isolation/expected/ri-cascade-del.out | 27 ++++++
>> > src/test/isolation/isolation_schedule | 1 +
>> > src/test/isolation/specs/ri-cascade-del.spec | 83 +++++++++++++++++++
>> > 3 files changed, 111 insertions(+)
>> > create mode 100644 src/test/isolation/expected/ri-cascade-del.out
>> > create mode 100644 src/test/isolation/specs/ri-cascade-del.spec
>> >
>> > diff --git a/src/test/isolation/expected/ri-cascade-del.out
>> b/src/test/isolation/expected/ri-cascade-del.out
>> > new file mode 100644
>> > index 00000000000..bd7ae0b0b5a
>> > --- /dev/null
>> > +++ b/src/test/isolation/expected/ri-cascade-del.out
>> > @@ -0,0 +1,27 @@
>> > +Parsed test spec with 2 sessions
>> > +
>> > +starting permutation: s2_lock s1_delete s2_inval s2_unlock
>> > +step s2_lock: SELECT pg_advisory_lock(0);
>> > +pg_advisory_lock
>> > +----------------
>> > +
>> > +(1 row)
>> > +
>> > +step s1_delete: DELETE FROM crash_reentrancia_tabla_autoreferencial
>> WHERE id = 1; <waiting ...>
>> > +step s2_inval:
>> > + DO $$
>> > + BEGIN
>> > + FOR i IN 1..1000 LOOP
>> > + EXECUTE 'CREATE TEMPORARY TABLE t_temp_inval_(id INTEGER PRIMARY
>> KEY)';
>> > + EXECUTE 'DROP TABLE t_temp_inval_';
>> > + END LOOP;
>> > + END;
>> > + $$;
>> > +
>> > +step s2_unlock: SELECT pg_advisory_unlock(0);
>> > +pg_advisory_unlock
>> > +------------------
>> > +t
>> > +(1 row)
>> > +
>> > +step s1_delete: <... completed>
>> > diff --git a/src/test/isolation/isolation_schedule
>> b/src/test/isolation/isolation_schedule
>> > index 1578ba191c8..39a0a1ee792 100644
>> > --- a/src/test/isolation/isolation_schedule
>> > +++ b/src/test/isolation/isolation_schedule
>> > @@ -12,6 +12,7 @@ test: project-manager
>> > test: classroom-scheduling
>> > test: total-cash
>> > test: referential-integrity
>> > +test: ri-cascade-del
>> > test: ri-trigger
>> > test: partial-index
>> > test: two-ids
>> > diff --git a/src/test/isolation/specs/ri-cascade-del.spec
>> b/src/test/isolation/specs/ri-cascade-del.spec
>> > new file mode 100644
>> > index 00000000000..c412ec2c772
>> > --- /dev/null
>> > +++ b/src/test/isolation/specs/ri-cascade-del.spec
>> > @@ -0,0 +1,83 @@
>> > +# Setup for referential integrity crash test
>> > +setup
>> > +{
>> > + CREATE TABLE crash_reentrancia_tabla_autoreferencial (
>> > + id int PRIMARY KEY,
>> > + nombre text,
>> > + padre_id int REFERENCES
>> crash_reentrancia_tabla_autoreferencial(id) ON DELETE CASCADE
>> > + );
>> > +
>> > + CREATE TABLE crash_reentrancia_segunda_tabla (
>> > + id int PRIMARY KEY,
>> > + valor text
>> > + );
>> > +
>> > + CREATE OR REPLACE FUNCTION crash_reentrancia_before_delete()
>> > + RETURNS trigger AS $$
>> > + DECLARE
>> > + v_valor text;
>> > + BEGIN
>> > + IF OLD.id % 2 = 1 THEN
>> > + RETURN OLD;
>> > + END IF;
>> > +
>> > + -- Wait for S2 to finish flooding the invalidation message
>> queue
>> > + IF OLD.id = 2 THEN
>> > + PERFORM pg_advisory_lock(0);
>> > + PERFORM pg_advisory_unlock(0);
>> > + END IF;
>> > +
>> > + IF OLD.id > 4 THEN
>> > + -- This opens the table and forces processing of pending
>> inval messages
>> > + SELECT valor INTO v_valor FROM
>> crash_reentrancia_segunda_tabla WHERE id = OLD.id;
>> > + END IF;
>> > +
>> > + DELETE FROM crash_reentrancia_tabla_autoreferencial WHERE
>> padre_id = OLD.id;
>> > + RETURN OLD;
>> > + END;
>> > + $$ LANGUAGE plpgsql;
>> > +
>> > + CREATE TRIGGER trg_crash_reentrancia_before_delete
>> > + BEFORE DELETE ON crash_reentrancia_tabla_autoreferencial
>> > + FOR EACH ROW EXECUTE FUNCTION
>> crash_reentrancia_before_delete();
>> > +
>> > + INSERT INTO crash_reentrancia_tabla_autoreferencial VALUES (1,
>> 'A', NULL);
>> > + INSERT INTO crash_reentrancia_tabla_autoreferencial VALUES (2,
>> 'B', 1);
>> > + INSERT INTO crash_reentrancia_tabla_autoreferencial VALUES (3,
>> 'C', 2);
>> > + INSERT INTO crash_reentrancia_tabla_autoreferencial VALUES (4,
>> 'D', 3);
>> > + INSERT INTO crash_reentrancia_tabla_autoreferencial VALUES (5,
>> 'E', 4);
>> > + INSERT INTO crash_reentrancia_tabla_autoreferencial VALUES (6,
>> 'F', 5);
>> > +
>> > + INSERT INTO crash_reentrancia_segunda_tabla VALUES
>> > + (1, 'a'), (2, 'b'), (3, 'c'), (4, 'd'), (5, 'e'), (6, 'f');
>> > +}
>> > +
>> > +teardown
>> > +{
>> > + DROP TRIGGER trg_crash_reentrancia_before_delete ON
>> crash_reentrancia_tabla_autoreferencial;
>> > + DROP FUNCTION crash_reentrancia_before_delete CASCADE;
>> > + DROP TABLE crash_reentrancia_tabla_autoreferencial CASCADE;
>> > + DROP TABLE crash_reentrancia_segunda_tabla CASCADE;
>> > +}
>> > +
>> > +session s1
>> > +step s1_delete { DELETE FROM crash_reentrancia_tabla_autoreferencial
>> WHERE id = 1; }
>> > +
>> > +session s2
>> > +step s2_lock { SELECT pg_advisory_lock(0); }
>> > +step s2_inval {
>> > + DO $$
>> > + BEGIN
>> > + FOR i IN 1..1000 LOOP
>> > + EXECUTE 'CREATE TEMPORARY TABLE t_temp_inval_(id INTEGER PRIMARY
>> KEY)';
>> > + EXECUTE 'DROP TABLE t_temp_inval_';
>> > + END LOOP;
>> > + END;
>> > + $$;
>> > +}
>> > +step s2_unlock { SELECT pg_advisory_unlock(0); }
>> > +
>> > +# Execution permutation
>> > +# S2 locks -> S1 blocks on S2 -> S2 forces inval queue overflow -> S2
>> unlocks
>> > +# S1 awakens -> S1 forces table_open -> invalidation processed ->
>> segfault!
>> > +permutation s2_lock s1_delete s2_inval s2_unlock
>> > --
>> > 2.34.1
>> >
>>
>> > From e8dfb066e7d49cc1550969871b43fac9ed01c04c Mon Sep 17 00:00:00 2001
>> > From: luquijeffrey <lucas(dot)jeffrey(at)anachronics(dot)com>
>> > Date: Fri, 29 May 2026 12:23:54 -0300
>> > Subject: [PATCH 2/2]
>> =?UTF-8?q?Fix=20crash=20of=20self=E2=80=91referencing?=
>> > =?UTF-8?q?=20tables=20with=20delete=20triggers?=
>> > MIME-Version: 1.0
>> > Content-Type: text/plain; charset=UTF-8
>> > Content-Transfer-Encoding: 8bit
>> >
>> > ---
>> > src/backend/utils/adt/ri_triggers.c | 130 +++++++++++++++++++++++++++-
>> > 1 file changed, 129 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/src/backend/utils/adt/ri_triggers.c
>> b/src/backend/utils/adt/ri_triggers.c
>> > index dc89c686394..425e7f9e1eb 100644
>> > --- a/src/backend/utils/adt/ri_triggers.c
>> > +++ b/src/backend/utils/adt/ri_triggers.c
>> > @@ -251,12 +251,25 @@ typedef struct RI_FastPathEntry
>> > int batch_count;
>> > } RI_FastPathEntry;
>> >
>> > +/*
>> > + * RI_QueryPlanCacheExecutingRefCountEntry
>> > + *
>> > + * Entry to track the number of times a prepared plan is being
>> executed.
>> > + */
>> > +typedef struct RI_QueryPlanCacheExecutingRefCountEntry
>> > +{
>> > + SPIPlanPtr plan;
>> > + bool markedForDeletion; /* If true, it will be freed when
>> refcount reaches 0 */
>> > + uint32 refcount; /* number of times this plan is being
>> executed (can be more than 1 if reentrant) */
>> > +} RI_QueryPlanCacheExecutingRefCountEntry;
>> > +
>> > /*
>> > * Local data
>> > */
>> > static HTAB *ri_constraint_cache = NULL;
>> > static HTAB *ri_query_cache = NULL;
>> > static HTAB *ri_compare_cache = NULL;
>> > +static HTAB *ri_query_plan_cache_executing_refcount = NULL;
>> > static dclist_head ri_constraint_cache_valid_list;
>> >
>> > static HTAB *ri_fastpath_cache = NULL;
>> > @@ -295,6 +308,11 @@ static SPIPlanPtr ri_FetchPreparedPlan(RI_QueryKey
>> *key);
>> > static void ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan);
>> > static RI_CompareHashEntry *ri_HashCompareOp(Oid eq_opr, Oid typeid);
>> >
>> > +/* Reentrancy protection: prevent segfault on deleting a plan in
>> execution if invalidated during reentrant RI check. */
>> > +static void ri_PreparedPlanExecutionStarted(SPIPlanPtr plan);
>> > +static void ri_PreparedPlanExecutionFinished(SPIPlanPtr plan);
>> > +static void ri_PreparedPlanReleaseASAP(SPIPlanPtr plan);
>> > +
>> > static void ri_CheckTrigger(FunctionCallInfo fcinfo, const char
>> *funcname,
>> > int tgkind);
>> > static RI_ConstraintInfo *ri_FetchConstraintInfo(Trigger *trigger,
>> > @@ -2724,6 +2742,9 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo,
>> > save_sec_context |
>> SECURITY_LOCAL_USERID_CHANGE |
>> > SECURITY_NOFORCE_RLS);
>> >
>> > + /* Increase plan use count for reentrancy protection. */
>> > + ri_PreparedPlanExecutionStarted(qplan);
>> > +
>> > /*
>> > * Finally we can run the query.
>> > *
>> > @@ -2735,6 +2756,9 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo,
>> >
>> vals, nulls,
>> >
>> test_snapshot, crosscheck_snapshot,
>> >
>> false, false, limit);
>> > +
>> > + /* Decrease plan use count. this call can free the plan if it was
>> invalidated and no longer in use. */
>> > + ri_PreparedPlanExecutionFinished(qplan);
>> >
>> > /* Restore UID and security context */
>> > SetUserIdAndSecContext(save_userid, save_sec_context);
>> > @@ -3762,6 +3786,12 @@ ri_InitHashTables(void)
>> > ri_compare_cache = hash_create("RI compare cache",
>> >
>> RI_INIT_QUERYHASHSIZE,
>> > &ctl,
>> HASH_ELEM | HASH_BLOBS);
>> > +
>> > + ctl.keysize = sizeof(SPIPlanPtr);
>> > + ctl.entrysize = sizeof(RI_QueryPlanCacheExecutingRefCountEntry);
>> > + ri_query_plan_cache_executing_refcount = hash_create("RI plan
>> cache execution refcount",
>> > +
>> RI_INIT_QUERYHASHSIZE,
>> > + &ctl,
>> HASH_ELEM | HASH_BLOBS);
>> > }
>> >
>> >
>> > @@ -3812,7 +3842,7 @@ ri_FetchPreparedPlan(RI_QueryKey *key)
>> > */
>> > entry->plan = NULL;
>> > if (plan)
>> > - SPI_freeplan(plan);
>> > + ri_PreparedPlanReleaseASAP(plan);
>> >
>> > return NULL;
>> > }
>> > @@ -3847,6 +3877,104 @@ ri_HashPreparedPlan(RI_QueryKey *key,
>> SPIPlanPtr plan)
>> > }
>> >
>> >
>> > +static void
>> > +ri_PreparedPlanExecutionStarted(SPIPlanPtr plan)
>> > +{
>> > + RI_QueryPlanCacheExecutingRefCountEntry* entry;
>> > + bool found;
>> > +
>> > + if (!ri_query_plan_cache_executing_refcount)
>> > + ri_InitHashTables();
>> > +
>> > + entry = (RI_QueryPlanCacheExecutingRefCountEntry*)
>> hash_search(ri_query_plan_cache_executing_refcount, &plan, HASH_ENTER,
>> &found);
>> > + if (found)
>> > + entry->refcount++;
>> > + else
>> > + {
>> > + entry->refcount = 1;
>> > + entry->markedForDeletion = false;
>> > + }
>> > +}
>> > +
>> > +static void
>> > +ri_PreparedPlanExecutionFinished(SPIPlanPtr plan)
>> > +{
>> > + RI_QueryPlanCacheExecutingRefCountEntry* entry;
>> > + bool found;
>> > +
>> > + if (!ri_query_plan_cache_executing_refcount)
>> > + return;
>> > +
>> > + entry = (RI_QueryPlanCacheExecutingRefCountEntry*)
>> hash_search(ri_query_plan_cache_executing_refcount, &plan, HASH_FIND,
>> &found);
>> > + if (!entry)
>> > + return;
>> > +
>> > + entry->refcount--;
>> > + if (entry->refcount == 0 && entry->markedForDeletion)
>> > + {
>> > + // Remove the entry
>> > + hash_search(ri_query_plan_cache_executing_refcount,
>> &plan, HASH_REMOVE, NULL);
>> > + SPI_freeplan(plan);
>> > + }
>> > +}
>> > +
>> > +/*
>> > + * ri_PreparedPlanReleaseASAP
>> > + *
>> > + * Release a cached SPI plan, or mark it for deferred deletion if it
>> > + * is currently in use.
>> > + *
>> > + * If the plan has an active executing-refcount entry with refcount >
>> 0,
>> > + * we cannot free it immediately. Instead we mark it for deletion so
>> > + * that the last executor to finish will free it.
>> > + */
>> > +static void
>> > +ri_PreparedPlanReleaseASAP(SPIPlanPtr plan)
>> > +{
>> > + RI_QueryPlanCacheExecutingRefCountEntry *entry;
>> > + bool found;
>> > +
>> > + /*
>> > + * If there is no executing-refcount hash table, it's not in use,
>> > + * so we can free immediately.
>> > + */
>> > + if (!ri_query_plan_cache_executing_refcount)
>> > + {
>> > + SPI_freeplan(plan);
>> > + return;
>> > + }
>> > +
>> > + entry = (RI_QueryPlanCacheExecutingRefCountEntry *)
>> > + hash_search(ri_query_plan_cache_executing_refcount,
>> > + &plan, HASH_FIND, &found);
>> > +
>> > + /*
>> > + * No refcount entry means the plan is not being executed; free
>> it now.
>> > + */
>> > + if (!found)
>> > + {
>> > + SPI_freeplan(plan);
>> > + return;
>> > + }
>> > +
>> > + /*
>> > + * If the refcount has dropped to zero, remove the entry and free
>> the
>> > + * plan. Otherwise mark it for deletion once the last executor
>> finishes.
>> > + */
>> > + if (entry->refcount == 0)
>> > + {
>> > + hash_search(ri_query_plan_cache_executing_refcount,
>> > + &plan, HASH_REMOVE, NULL);
>> > + SPI_freeplan(plan);
>> > + return;
>> > + }
>> > +
>> > + /*
>> > + * Mark for deletion once the last executor finishes.
>> > + */
>> > + entry->markedForDeletion = true;
>> > +}
>> > +
>> > /*
>> > * ri_KeysEqual -
>> > *
>> > --
>> > 2.34.1
>> >
>>
>>
>>
>> --
>> Álvaro Herrera 48°01'N 7°57'E —
>> https://www.EnterpriseDB.com/
>> "La espina, desde que nace, ya pincha" (Proverbio africano)
>>
>