From: Amit Langote Date: Fri, 12 Jun 2026 01:36:45 +0000 (+0900) Subject: Fix out-of-bounds write in RI fast-path batch on re-entry X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=0e47bb5fbeec64d776d49dee242bac39d4616f8b;p=thirdparty%2Fpostgresql.git Fix out-of-bounds write in RI fast-path batch on re-entry The FK fast-path batching added in b7b27eb41a5 wrote the incoming row into the batch array before checking whether the array was full: fpentry->batch[fpentry->batch_count] = ExecCopySlotHeapTuple(newslot); fpentry->batch_count++; if (fpentry->batch_count >= RI_FASTPATH_BATCH_SIZE) ri_FastPathBatchFlush(fpentry, fk_rel, riinfo); batch_count is reset to zero only at the end of ri_FastPathBatchFlush(), so it remains at RI_FASTPATH_BATCH_SIZE throughout a full-batch flush. A flush runs user-defined cast functions and equality operators; if that user code performs DML on the same FK table, ri_FastPathBatchAdd() re-enters with batch_count == RI_FASTPATH_BATCH_SIZE and writes one past the end of the array, corrupting the adjacent batch_count field. This is reachable by an unprivileged table owner via an implicit cast with a PL/pgSQL function and causes a SIGSEGV in assert-enabled builds. Fix by bounds-checking the write into the batch array so a re-entrant add can never write past the end, and by adding a "flushing" flag to RI_FastPathEntry that routes re-entrant ri_FastPathBatchAdd() calls on a busy entry to the per-row path (ri_FastPathCheck) instead of touching the mid-flush batch array. The flag is set around the probe in ri_FastPathBatchFlush() and cleared in a PG_FINALLY, which also resets batch_count, so the entry is left empty and reusable if a flush error (including a reported FK violation) is caught by a savepoint. Add regression tests for both the re-entrant flush and reuse of an entry after a flush error caught by a savepoint. Reported-by: Nikolay Samokhvalov Reviewed-by: Nikolay Samokhvalov Reviewed-by: Ayush Tiwari Reviewed-by: Junwang Zhao Discussion: https://postgr.es/m/CAM527d9exRCdWrhJOnAxk_vACg7sr_yPoaJp_+uCFY0qP8v=aw@mail.gmail.com --- diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index dc89c686394..917a544c32b 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -249,6 +249,12 @@ typedef struct RI_FastPathEntry */ HeapTuple batch[RI_FASTPATH_BATCH_SIZE]; int batch_count; + + /* + * true while this entry's batch is being flushed; guards against + * re-entrant ri_FastPathBatchAdd from user code run during the flush. + */ + bool flushing; } RI_FastPathEntry; /* @@ -2860,15 +2866,38 @@ ri_FastPathBatchAdd(RI_ConstraintInfo *riinfo, Relation fk_rel, TupleTableSlot *newslot) { RI_FastPathEntry *fpentry = ri_FastPathGetEntry(riinfo, fk_rel); - MemoryContext oldcxt; - oldcxt = MemoryContextSwitchTo(fpentry->flush_cxt); - fpentry->batch[fpentry->batch_count] = - ExecCopySlotHeapTuple(newslot); - fpentry->batch_count++; - MemoryContextSwitchTo(oldcxt); + /* + * If this entry is already being flushed, a cast function or an operator + * invoked during the flush has re-entered with DML on the same FK. Fall + * back to the per-row path rather than touching the batch array, which is + * mid-flush. + */ + if (unlikely(fpentry->flushing)) + { + ri_FastPathCheck(riinfo, fk_rel, newslot); + return; + } + + /* + * Buffer the row. A full batch is flushed below and re-entry is handled + * above, so there is always room here; the bounds check just guards the + * array write. + */ + if (fpentry->batch_count < RI_FASTPATH_BATCH_SIZE) + { + MemoryContext oldcxt = MemoryContextSwitchTo(fpentry->flush_cxt); - if (fpentry->batch_count >= RI_FASTPATH_BATCH_SIZE) + fpentry->batch[fpentry->batch_count] = + ExecCopySlotHeapTuple(newslot); + fpentry->batch_count++; + MemoryContextSwitchTo(oldcxt); + } + else + elog(ERROR, "RI fast-path batch unexpectedly full"); + + /* Flush as soon as the batch is full. */ + if (fpentry->batch_count == RI_FASTPATH_BATCH_SIZE) ri_FastPathBatchFlush(fpentry, fk_rel, riinfo); } @@ -2944,13 +2973,30 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel, } Assert(riinfo->fpmeta); - /* Skip array overhead for single-row batches. */ - if (riinfo->nkeys == 1 && fpentry->batch_count > 1) - violation_index = ri_FastPathFlushArray(fpentry, fk_slot, riinfo, - fk_rel, snapshot, scandesc); - else - violation_index = ri_FastPathFlushLoop(fpentry, fk_slot, riinfo, - fk_rel, snapshot, scandesc); + /* + * The probe runs user-defined cast and equality functions. Set the + * flushing flag around it so a re-entrant ri_FastPathBatchAdd on this + * entry takes the per-row path, and clear it even on error so the entry + * is reusable if the error is caught by a savepoint. + */ + Assert(!fpentry->flushing); + fpentry->flushing = true; + PG_TRY(); + { + /* Skip array overhead for single-row batches. */ + if (riinfo->nkeys == 1 && fpentry->batch_count > 1) + violation_index = ri_FastPathFlushArray(fpentry, fk_slot, riinfo, + fk_rel, snapshot, scandesc); + else + violation_index = ri_FastPathFlushLoop(fpentry, fk_slot, riinfo, + fk_rel, snapshot, scandesc); + } + PG_FINALLY(); + { + fpentry->flushing = false; + fpentry->batch_count = 0; + } + PG_END_TRY(); SetUserIdAndSecContext(saved_userid, saved_sec_context); UnregisterSnapshot(snapshot); @@ -2966,9 +3012,6 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel, MemoryContextReset(fpentry->flush_cxt); MemoryContextSwitchTo(oldcxt); - - /* Reset. */ - fpentry->batch_count = 0; } /* @@ -4307,6 +4350,9 @@ ri_FastPathGetEntry(const RI_ConstraintInfo *riinfo, Relation fk_rel) RegisterAfterTriggerBatchCallback(ri_FastPathEndBatch, NULL); ri_fastpath_callback_registered = true; } + + entry->flushing = false; + entry->batch_count = 0; } return entry; diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 8b3b268de0f..e08dff99f03 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -3712,3 +3712,59 @@ INSERT INTO fp_pk_dup VALUES (1); CREATE TABLE fp_fk_dup (a int REFERENCES fp_pk_dup); INSERT INTO fp_fk_dup SELECT 1 FROM generate_series(1, 100); DROP TABLE fp_fk_dup, fp_pk_dup; +-- Re-entrant FK fast-path: DML on the same FK table from a cast function +-- during a full-batch flush must not corrupt the batch array. +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)); +-- Fill exactly one batch so the flush fires; the cast re-enters with DML +-- on the same FK and must take the per-row path. +INSERT INTO fp_reentry_fk SELECT row(1)::fp_vch FROM generate_series(1, 64); +SELECT a, count(*) FROM fp_reentry_fk GROUP BY a ORDER BY a; + a | count +-----+------- + (1) | 64 + (2) | 64 +(2 rows) + +DROP TABLE fp_reentry_fk, fp_reentry_pk; +DROP CAST (fp_vch AS int); +DROP FUNCTION fp_vcast(fp_vch); +DROP TYPE fp_vch; +-- Flush error caught by a savepoint must leave the entry empty and reusable. +CREATE TABLE fp_reentry_pk2 (id int PRIMARY KEY); +INSERT INTO fp_reentry_pk2 VALUES (1); +CREATE TABLE fp_reentry_fk2 (a int REFERENCES fp_reentry_pk2 (id)); +DO $$ +BEGIN + -- A batch containing a violating row; the flush reports the violation. + BEGIN + INSERT INTO fp_reentry_fk2 SELECT CASE WHEN g = 32 THEN 999 ELSE 1 END + FROM generate_series(1, 64) g; + EXCEPTION WHEN foreign_key_violation THEN + RAISE NOTICE 'caught fk violation'; + END; + + -- Reuse the same FK with a full batch in the same transaction. The + -- entry must be empty after the caught violation: no stale rows from the + -- rolled-back batch (in particular no 999), and no array overflow. + INSERT INTO fp_reentry_fk2 SELECT 1 FROM generate_series(1, 64); +END$$; +NOTICE: caught fk violation +SELECT count(*), max(a) FROM fp_reentry_fk2; -- 64 rows, max 1 + count | max +-------+----- + 64 | 1 +(1 row) + +DROP TABLE fp_reentry_fk2, fp_reentry_pk2; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 7eb86b188f0..87381194f41 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2680,3 +2680,49 @@ INSERT INTO fp_pk_dup VALUES (1); CREATE TABLE fp_fk_dup (a int REFERENCES fp_pk_dup); INSERT INTO fp_fk_dup SELECT 1 FROM generate_series(1, 100); DROP TABLE fp_fk_dup, fp_pk_dup; + +-- Re-entrant FK fast-path: DML on the same FK table from a cast function +-- during a full-batch flush must not corrupt the batch array. +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)); +-- Fill exactly one batch so the flush fires; the cast re-enters with DML +-- on the same FK and must take the per-row path. +INSERT INTO fp_reentry_fk SELECT row(1)::fp_vch FROM generate_series(1, 64); +SELECT a, count(*) FROM fp_reentry_fk GROUP BY a ORDER BY a; +DROP TABLE fp_reentry_fk, fp_reentry_pk; +DROP CAST (fp_vch AS int); +DROP FUNCTION fp_vcast(fp_vch); +DROP TYPE fp_vch; + +-- Flush error caught by a savepoint must leave the entry empty and reusable. +CREATE TABLE fp_reentry_pk2 (id int PRIMARY KEY); +INSERT INTO fp_reentry_pk2 VALUES (1); +CREATE TABLE fp_reentry_fk2 (a int REFERENCES fp_reentry_pk2 (id)); +DO $$ +BEGIN + -- A batch containing a violating row; the flush reports the violation. + BEGIN + INSERT INTO fp_reentry_fk2 SELECT CASE WHEN g = 32 THEN 999 ELSE 1 END + FROM generate_series(1, 64) g; + EXCEPTION WHEN foreign_key_violation THEN + RAISE NOTICE 'caught fk violation'; + END; + + -- Reuse the same FK with a full batch in the same transaction. The + -- entry must be empty after the caught violation: no stale rows from the + -- rolled-back batch (in particular no 999), and no array overflow. + INSERT INTO fp_reentry_fk2 SELECT 1 FROM generate_series(1, 64); +END$$; +SELECT count(*), max(a) FROM fp_reentry_fk2; -- 64 rows, max 1 +DROP TABLE fp_reentry_fk2, fp_reentry_pk2;