Fix use-after-free bug with AfterTriggersTableData.storeslot

AfterTriggerSaveEvent() wrongly allocates the slot in execution-span
memory context, whereas the correct thing is to allocate it in
a transaction-span context, because that's where the enclosing
AfterTriggersTableData instance belongs into.

Backpatch to 12 (the test back to 11, where it works well with no code
changes, and it's good to have to confirm that the case was previously
well supported); this bug seems introduced by commit ff11e7f4b9ae.

Reported-by: Bertrand Drouvot 
Author: Amit Langote 
Discussion: https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 8908847c6c690ed6a35770c6076a92989979eae7..4e4e05844c5b00339a0a7656e3dbdff694ed2c1f 100644 (file)
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3536,6 +3536,8 @@ static void AfterTriggerExecute(EState *estate,
                                TupleTableSlot *trig_tuple_slot2);
 static AfterTriggersTableData *GetAfterTriggersTableData(Oid relid,
                                                         CmdType cmdType);
+static TupleTableSlot *GetAfterTriggersStoreSlot(AfterTriggersTableData *table,
+                                                TupleDesc tupdesc);
 static void AfterTriggerFreeQuery(AfterTriggersQueryData *qs);
 static SetConstraintState SetConstraintStateCreate(int numalloc);
 static SetConstraintState SetConstraintStateCopy(SetConstraintState state);
@@ -4336,6 +4338,31 @@ GetAfterTriggersTableData(Oid relid, CmdType cmdType)
    return table;
 }
 
+/*
+ * Returns a TupleTableSlot suitable for holding the tuples to be put
+ * into AfterTriggersTableData's transition table tuplestores.
+ */
+static TupleTableSlot *
+GetAfterTriggersStoreSlot(AfterTriggersTableData *table,
+                         TupleDesc tupdesc)
+{
+   /* Create it if not already done. */
+   if (!table->storeslot)
+   {
+       MemoryContext   oldcxt;
+
+       /*
+        * We only need this slot only until AfterTriggerEndQuery, but making
+        * it last till end-of-subxact is good enough.  It'll be freed by
+        * AfterTriggerFreeQuery().
+        */
+       oldcxt = MemoryContextSwitchTo(CurTransactionContext);
+       table->storeslot = MakeSingleTupleTableSlot(tupdesc, &TTSOpsVirtual);
+       MemoryContextSwitchTo(oldcxt);
+   }
+
+   return table->storeslot;
+}
 
 /*
  * MakeTransitionCaptureState
@@ -4625,6 +4652,8 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs)
        table->new_tuplestore = NULL;
        if (ts)
            tuplestore_end(ts);
+       if (table->storeslot)
+           ExecDropSingleTupleTableSlot(table->storeslot);
    }
 
    /*
@@ -5474,17 +5503,10 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
 
            if (map != NULL)
            {
+               AfterTriggersTableData *table = transition_capture->tcs_private;
                TupleTableSlot *storeslot;
 
-               storeslot = transition_capture->tcs_private->storeslot;
-               if (!storeslot)
-               {
-                   storeslot = ExecAllocTableSlot(&estate->es_tupleTable,
-                                                  map->outdesc,
-                                                  &TTSOpsVirtual);
-                   transition_capture->tcs_private->storeslot = storeslot;
-               }
-
+               storeslot = GetAfterTriggersStoreSlot(table, map->outdesc);
                execute_attr_map_slot(map->attrMap, oldslot, storeslot);
                tuplestore_puttupleslot(old_tuplestore, storeslot);
            }
@@ -5504,18 +5526,10 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
                                        original_insert_tuple);
            else if (map != NULL)
            {
+               AfterTriggersTableData *table = transition_capture->tcs_private;
                TupleTableSlot *storeslot;
 
-               storeslot = transition_capture->tcs_private->storeslot;
-
-               if (!storeslot)
-               {
-                   storeslot = ExecAllocTableSlot(&estate->es_tupleTable,
-                                                  map->outdesc,
-                                                  &TTSOpsVirtual);
-                   transition_capture->tcs_private->storeslot = storeslot;
-               }
-
+               storeslot = GetAfterTriggersStoreSlot(table, map->outdesc);
                execute_attr_map_slot(map->attrMap, newslot, storeslot);
                tuplestore_puttupleslot(new_tuplestore, storeslot);
            }

diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index b263002293bb8f1a50e4e16c597a03d73aff6e0b..e8af9a9589f57037906a3ac4a3125cccdbabb2fa 100644 (file)
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -3290,3 +3290,62 @@ create trigger aft_row after insert or update on trigger_parted
 create table trigger_parted_p1 partition of trigger_parted for values in (1)
   partition by list (a);
 create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
+-- verify transition table conversion slot's lifetime
+-- https://postgr.es/m/[email protected]
+create table convslot_test_parent (col1 text primary key);
+create table convslot_test_child (col1 text primary key,
+   foreign key (col1) references convslot_test_parent(col1) on delete cascade on update cascade
+);
+alter table convslot_test_child add column col2 text not null default 'tutu';
+insert into convslot_test_parent(col1) values ('1');
+insert into convslot_test_child(col1) values ('1');
+insert into convslot_test_parent(col1) values ('3');
+insert into convslot_test_child(col1) values ('3');
+create or replace function trigger_function1()
+returns trigger
+language plpgsql
+AS $$
+begin
+raise notice 'trigger = %, old_table = %',
+          TG_NAME,
+          (select string_agg(old_table::text, ', ' order by col1) from old_table);
+return null;
+end; $$;
+create or replace function trigger_function2()
+returns trigger
+language plpgsql
+AS $$
+begin
+raise notice 'trigger = %, new table = %',
+          TG_NAME,
+          (select string_agg(new_table::text, ', ' order by col1) from new_table);
+return null;
+end; $$;
+create trigger but_trigger after update on convslot_test_child
+referencing new table as new_table
+for each statement execute function trigger_function2();
+update convslot_test_parent set col1 = col1 || '1';
+NOTICE:  trigger = but_trigger, new table = (11,tutu), (31,tutu)
+create or replace function trigger_function3()
+returns trigger
+language plpgsql
+AS $$
+begin
+raise notice 'trigger = %, old_table = %, new table = %',
+          TG_NAME,
+          (select string_agg(old_table::text, ', ' order by col1) from old_table),
+          (select string_agg(new_table::text, ', ' order by col1) from new_table);
+return null;
+end; $$;
+create trigger but_trigger2 after update on convslot_test_child
+referencing old table as old_table new table as new_table
+for each statement execute function trigger_function3();
+update convslot_test_parent set col1 = col1 || '1';
+NOTICE:  trigger = but_trigger, new table = (111,tutu), (311,tutu)
+NOTICE:  trigger = but_trigger2, old_table = (11,tutu), (31,tutu), new table = (111,tutu), (311,tutu)
+create trigger bdt_trigger after delete on convslot_test_child
+referencing old table as old_table
+for each statement execute function trigger_function1();
+delete from convslot_test_parent;
+NOTICE:  trigger = bdt_trigger, old_table = (111,tutu), (311,tutu)
+drop table convslot_test_child, convslot_test_parent;

diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 01f66af699cb4a7cc85d865148c020f2b5491474..b50f500045b3b14e19ee6eaeceead18b93a89d27 100644 (file)
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -2470,3 +2470,68 @@ create trigger aft_row after insert or update on trigger_parted
 create table trigger_parted_p1 partition of trigger_parted for values in (1)
   partition by list (a);
 create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
+
+-- verify transition table conversion slot's lifetime
+-- https://postgr.es/m/[email protected]
+create table convslot_test_parent (col1 text primary key);
+create table convslot_test_child (col1 text primary key,
+   foreign key (col1) references convslot_test_parent(col1) on delete cascade on update cascade
+);
+
+alter table convslot_test_child add column col2 text not null default 'tutu';
+insert into convslot_test_parent(col1) values ('1');
+insert into convslot_test_child(col1) values ('1');
+insert into convslot_test_parent(col1) values ('3');
+insert into convslot_test_child(col1) values ('3');
+
+create or replace function trigger_function1()
+returns trigger
+language plpgsql
+AS $$
+begin
+raise notice 'trigger = %, old_table = %',
+          TG_NAME,
+          (select string_agg(old_table::text, ', ' order by col1) from old_table);
+return null;
+end; $$;
+
+create or replace function trigger_function2()
+returns trigger
+language plpgsql
+AS $$
+begin
+raise notice 'trigger = %, new table = %',
+          TG_NAME,
+          (select string_agg(new_table::text, ', ' order by col1) from new_table);
+return null;
+end; $$;
+
+create trigger but_trigger after update on convslot_test_child
+referencing new table as new_table
+for each statement execute function trigger_function2();
+
+update convslot_test_parent set col1 = col1 || '1';
+
+create or replace function trigger_function3()
+returns trigger
+language plpgsql
+AS $$
+begin
+raise notice 'trigger = %, old_table = %, new table = %',
+          TG_NAME,
+          (select string_agg(old_table::text, ', ' order by col1) from old_table),
+          (select string_agg(new_table::text, ', ' order by col1) from new_table);
+return null;
+end; $$;
+
+create trigger but_trigger2 after update on convslot_test_child
+referencing old table as old_table new table as new_table
+for each statement execute function trigger_function3();
+update convslot_test_parent set col1 = col1 || '1';
+
+create trigger bdt_trigger after delete on convslot_test_child
+referencing old table as old_table
+for each statement execute function trigger_function1();
+delete from convslot_test_parent;
+
+drop table convslot_test_child, convslot_test_parent;