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 (<[email protected]>) escribió: > Hi Alvaro, > > On Fri, Aug 14, 2026 at 21:37 Álvaro Herrera <[email protected]> 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_v...@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 (< >> > [email protected]>) 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 <[email protected]> >> > 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 <[email protected]> >> > 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) >> >
