| From: | Lucas Jeffrey <luquijeffrey(at)gmail(dot)com> |
|---|---|
| To: | Amit Langote <amitlangote09(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Two more RI fast-path issues |
| Date: | 2026-10-20 00:49:16 |
| Message-ID: | CAObkL7NeQPxj7SBi3u5k=MKtaZUPoqgyehiwug626sjD2fo9ZQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Amit
About the reentrant RI check that frees a plan already in use, in this
thread I already created a fix and it had a backfix for all supported
branches:
https://www.postgresql.org/message-id/flat/CAGHzy7S0XfXdK3amd5c4QhpS1o9dySVzPr+-d9ohaE4iV_vNog(at)mail(dot)gmail(dot)com
It has this entry in the current commitfest:
https://commitfest.postgresql.org/patch/6825/
On Mon, Oct 19, 2026 at 9:27 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> On Sat, Oct 17, 2026 at 10:36 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Sat, Oct 17, 2026 at 12:17 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > >
> > > There is still something wrong in the RI_FKey stuff. BF animal
> > > trilobite reported a SIGSEGV in the foreign_key test, and I was able
> > > to reproduce that locally by compiling HEAD with -DCLOBBER_CACHE_ALWAYS.
> > > Stack trace looks like
> > >
> > > Program terminated with signal SIGSEGV, Segmentation fault.
> > > #0 0x00000000007770df in _SPI_execute_plan (plan=plan(at)entry=0x184fafc0, options=options(at)entry=0x7fff7819da00, snapshot=snapshot(at)entry=0x0, crosscheck_snapshot=crosscheck_snapshot(at)entry=0x0,
> > > fire_triggers=fire_triggers(at)entry=false) at spi.c:2498
> > > 2498 CachedPlanSource *plansource = (CachedPlanSource *) lfirst(lc1);
> > > (gdb) bt
> > > #0 0x00000000007770df in _SPI_execute_plan (plan=plan(at)entry=0x184fafc0, options=options(at)entry=0x7fff7819da00, snapshot=snapshot(at)entry=0x0, crosscheck_snapshot=crosscheck_snapshot(at)entry=0x0,
> > > fire_triggers=fire_triggers(at)entry=false) at spi.c:2498
> > > #1 0x0000000000778030 in SPI_execute_snapshot (plan=plan(at)entry=0x184fafc0, Values=Values(at)entry=0x7fff7819db00, Nulls=Nulls(at)entry=0x7fff7819dac0 " ", snapshot=snapshot(at)entry=0x0,
> > > crosscheck_snapshot=crosscheck_snapshot(at)entry=0x0, read_only=read_only(at)entry=false, fire_triggers=false, tcount=1) at spi.c:796
> > > #2 0x0000000000a1565c in ri_PerformCheck (riinfo=riinfo(at)entry=0x183d6738, qkey=qkey(at)entry=0x7fff7819ddb8, qplan=0x184fafc0, fk_rel=fk_rel(at)entry=0x7f4431f051d0, pk_rel=pk_rel(at)entry=0x7f4431f08190,
> > > oldslot=oldslot(at)entry=0x0, newslot=0x1847e5a8, is_restrict=false, detectNewRows=false, expect_OK=5) at ri_triggers.c:2723
> > > #3 0x0000000000a16026 in RI_FKey_check (trigdata=<optimized out>) at ri_triggers.c:535
> > > #4 0x0000000000a17cdc in RI_FKey_check_ins (fcinfo=0x7fff7819e980) at ri_triggers.c:563
> > > #5 0x0000000000702b6d in ExecCallTriggerFunc (trigdata=trigdata(at)entry=0x7fff7819eb50, tgindx=tgindx(at)entry=0, finfo=finfo(at)entry=0x1821a620, instr=instr(at)entry=0x0,
> > > per_tuple_context=per_tuple_context(at)entry=0x18198a80) at trigger.c:2409
> > > #6 0x0000000000704ed6 in AfterTriggerExecute (estate=<optimized out>, event=0x181a2bec, relInfo=0x1847c9c8, src_relInfo=<optimized out>, dst_relInfo=<optimized out>, trigdesc=0x1821a810,
> > > finfo=0x1821a620, instr=0x0, per_tuple_context=<optimized out>, trig_tuple_slot1=0x0, trig_tuple_slot2=0x0) at trigger.c:4599
> > > #7 afterTriggerInvokeEvents (events=events(at)entry=0x180523b0, firing_id=1, estate=estate(at)entry=0x18218960, delete_ok=delete_ok(at)entry=false) at trigger.c:4840
> > > #8 0x000000000070a01b in AfterTriggerEndQuery (estate=estate(at)entry=0x18218960) at trigger.c:5222
> > > #9 0x000000000072ff34 in standard_ExecutorFinish (queryDesc=0x18482ac0) at execMain.c:454
> > > #10 0x000000000092f0d8 in ProcessQuery (plan=<optimized out>, sourceText=0x17fbc450 "INSERT INTO fp_reentry_fk SELECT row(1)::fp_vch FROM generate_series(1, 2);", params=0x0, queryEnv=0x0,
> > > dest=0x184a2ba8, qc=0x7fff7819ee90) at pquery.c:190
> > > #11 0x000000000092fbf6 in PortalRunMulti (portal=portal(at)entry=0x18044070, isTopLevel=isTopLevel(at)entry=true, setHoldSnapshot=setHoldSnapshot(at)entry=false, dest=dest(at)entry=0x184a2ba8,
> > > altdest=altdest(at)entry=0x184a2ba8, qc=qc(at)entry=0x7fff7819ee90) at pquery.c:1273
> > > #12 0x000000000092ffb7 in PortalRun (portal=portal(at)entry=0x18044070, count=count(at)entry=9223372036854775807, isTopLevel=isTopLevel(at)entry=true, dest=dest(at)entry=0x184a2ba8,
> > > altdest=altdest(at)entry=0x184a2ba8, qc=qc(at)entry=0x7fff7819ee90) at pquery.c:788
> > > #13 0x000000000092be16 in exec_simple_query (query_string=0x17fbc450 "INSERT INTO fp_reentry_fk SELECT row(1)::fp_vch FROM generate_series(1, 2);") at postgres.c:1297
> > > #14 0x000000000092d9ad in PostgresMain (dbname=<optimized out>, username=<optimized out>) at postgres.c:4946
> > > #15 0x0000000000927d5d in BackendMain (startup_data=<optimized out>, startup_data_len=<optimized out>) at backend_startup.c:124
> > > #16 0x000000000086fb7d in postmaster_child_launch (child_type=<optimized out>, child_slot=4, startup_data=startup_data(at)entry=0x7fff7819f300, startup_data_len=startup_data_len(at)entry=24,
> > > client_sock=client_sock(at)entry=0x7fff7819f320) at launch_backend.c:268
> > > #17 0x000000000087331f in BackendStartup (client_sock=0x7fff7819f320) at postmaster.c:3650
> > > #18 ServerLoop () at postmaster.c:1727
> > > #19 0x0000000000874c0a in PostmasterMain (argc=argc(at)entry=3, argv=argv(at)entry=0x17fb7e50) at postmaster.c:1414
> > > #20 0x0000000000532488 in main (argc=3, argv=0x17fb7e50) at main.c:227
> > > (gdb) p debug_query_string
> > > $1 = 0x17fbc450 "INSERT INTO fp_reentry_fk SELECT row(1)::fp_vch FROM generate_series(1, 2);"
> > >
> > > It looks like something is not prepared for a cached plan to get
> > > invalidated underneath it.
> >
> > Thanks for the heads up. Will debug this on Monday.
>
> I was able to reproduce this by adding set debug_discard_caches=1
> before the test case seen in the above trace.
>
> This is a use-after-free of a cached SPI plan in ri_triggers.c.
> ri_FetchPreparedPlan() frees a plan it finds invalid without checking
> whether an RI check further up the stack is still executing it, and
> CLOBBER_CACHE_ALWAYS makes that happen in the fp_reentry test.
>
> In that test case that I added to test the re-entrancy safety of the
> fast-path code new in 19, a user-defined cast function running as a
> part of the foreign key check itself performs an insert on the same
> table thus re-entering RI_FKey_check(), which calls
> ri_FetchPreparedPlan() for the same query key and so lands on the plan
> the outer check is executing. Here’s the test, with a SET added before
> the INSERT from your stack trace to get the CLOBBER_CACHE_ALWAYS
> effect:
>
> CREATE TABLE fp_reentry_pk (id int PRIMARY KEY);
> INSERT INTO fp_reentry_pk VALUES (1), (2);
> CREATE TYPE fp_vch AS (v int);
> CREATE FUNCTION fp_vcast(fp_vch) RETURNS int LANGUAGE plpgsql AS $$
> BEGIN
> IF $1.v = 1 THEN
> INSERT INTO fp_reentry_fk VALUES (row(2)::fp_vch);
> END IF;
> RETURN $1.v;
> END$$;
> CREATE CAST (fp_vch AS int) WITH FUNCTION fp_vcast(fp_vch) AS IMPLICIT;
> CREATE TABLE fp_reentry_fk (a fp_vch REFERENCES fp_reentry_pk (id));
>
> SET debug_discard_caches=1;
> INSERT INTO fp_reentry_fk SELECT row(1)::fp_vch FROM generate_series(1, 2);
>
> The inner check's ri_FetchPreparedPlan() sees under
> CLOBBER_CACHE_ALWAYS that the plan is invalid and calls SPI_freeplan()
> on it. The inner check completes using a fresh plan, but when the
> control returns to the outer check in _SPI_execute_plan(), its loop
> over plan->plancache_list reads the List header from freed memory
> (0x7f-filled under CLOBBER_FREED_MEMORY), takes the garbage length to
> mean there is a second cell, and segfaults dereferencing it. Any
> invalidation of the plan between the outer check’s fetch and the inner
> check’s fetch would have the same effect (cache clobbering just
> guarantees one).
>
> The bug itself is old. However, before 6aecabcd5ed5, in which I
> changed the foreign key checks that need to call cast functions to go
> through SPI, this test used the fast-path code and thus didn't hit
> this bug. In fact, the fast-path had an analogous UAF bug, which I
> fixed in 170c9344def. That bug occurred when an outer check was using
> a FastPathMeta struct that InvalidateConstraintCacheCallBack() freed.
>
> To fix, we can teach ri_FetchPreparedPlan() to “detach” a stale plan
> from its ri_query_cache entry (by doing entry->plan = NULL), so a new
> one can be put in place under the same key, and instead of calling
> SPI_freeplan() append it to a list that is cleaned up at end of
> transaction, when nothing on the stack can be referring to it. This
> follows the model I used in 170c9344def, where invalidated
> FastPathMeta structs are detached from their RI_ConstraintInfo and
> added to a list that AtEOXact_RI() processes.
>
> Patch for master attached. With it, the fp_reentry test case shown
> above passes with debug_discard_caches=1. I’ve left the test as is but
> added a comment mentioning that it exercises this under cache
> clobbering, which is what found it.
>
> This needs to go to all supported branches. AtEOXact_RI() exists only
> in 19 and master; for the older ones I’d add it with just the plan
> cleanup, so the code matches 19, rather than install a
> RegisterXactCallback hook (which would also need to cover PREPARE, not
> just COMMIT and ABORT).
>
> Thoughts?
>
> --
> Thanks, Amit Langote
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Afrah Razzak | 2026-10-20 00:56:34 | [PATCH] doc: vacuum_max_eager_freeze_failure_rate can be set by any user |
| Previous Message | Amit Langote | 2026-10-20 00:26:40 | Re: Two more RI fast-path issues |