Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 2688852

Browse files
committed
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 ff11e7f. Reported-by: Bertrand Drouvot <bdrouvot@amazon.com> Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com
1 parent 5744931 commit 2688852

File tree

3 files changed

+157
-19
lines changed

3 files changed

+157
-19
lines changed

src/backend/commands/trigger.c

+33-19
Original file line numberDiff line numberDiff line change
@@ -3461,6 +3461,8 @@ static void AfterTriggerExecute(EState *estate,
34613461
TupleTableSlot *trig_tuple_slot2);
34623462
static AfterTriggersTableData *GetAfterTriggersTableData(Oid relid,
34633463
CmdType cmdType);
3464+
static TupleTableSlot *GetAfterTriggersStoreSlot(AfterTriggersTableData *table,
3465+
TupleDesc tupdesc);
34643466
static void AfterTriggerFreeQuery(AfterTriggersQueryData *qs);
34653467
static SetConstraintState SetConstraintStateCreate(int numalloc);
34663468
static SetConstraintState SetConstraintStateCopy(SetConstraintState state);
@@ -4261,6 +4263,31 @@ GetAfterTriggersTableData(Oid relid, CmdType cmdType)
42614263
return table;
42624264
}
42634265

4266+
/*
4267+
* Returns a TupleTableSlot suitable for holding the tuples to be put
4268+
* into AfterTriggersTableData's transition table tuplestores.
4269+
*/
4270+
static TupleTableSlot *
4271+
GetAfterTriggersStoreSlot(AfterTriggersTableData *table,
4272+
TupleDesc tupdesc)
4273+
{
4274+
/* Create it if not already done. */
4275+
if (!table->storeslot)
4276+
{
4277+
MemoryContext oldcxt;
4278+
4279+
/*
4280+
* We only need this slot only until AfterTriggerEndQuery, but making
4281+
* it last till end-of-subxact is good enough. It'll be freed by
4282+
* AfterTriggerFreeQuery().
4283+
*/
4284+
oldcxt = MemoryContextSwitchTo(CurTransactionContext);
4285+
table->storeslot = MakeSingleTupleTableSlot(tupdesc, &TTSOpsVirtual);
4286+
MemoryContextSwitchTo(oldcxt);
4287+
}
4288+
4289+
return table->storeslot;
4290+
}
42644291

42654292
/*
42664293
* MakeTransitionCaptureState
@@ -4549,6 +4576,8 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs)
45494576
table->new_tuplestore = NULL;
45504577
if (ts)
45514578
tuplestore_end(ts);
4579+
if (table->storeslot)
4580+
ExecDropSingleTupleTableSlot(table->storeslot);
45524581
}
45534582

45544583
/*
@@ -5398,17 +5427,10 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
53985427

53995428
if (map != NULL)
54005429
{
5430+
AfterTriggersTableData *table = transition_capture->tcs_private;
54015431
TupleTableSlot *storeslot;
54025432

5403-
storeslot = transition_capture->tcs_private->storeslot;
5404-
if (!storeslot)
5405-
{
5406-
storeslot = ExecAllocTableSlot(&estate->es_tupleTable,
5407-
map->outdesc,
5408-
&TTSOpsVirtual);
5409-
transition_capture->tcs_private->storeslot = storeslot;
5410-
}
5411-
5433+
storeslot = GetAfterTriggersStoreSlot(table, map->outdesc);
54125434
execute_attr_map_slot(map->attrMap, oldslot, storeslot);
54135435
tuplestore_puttupleslot(old_tuplestore, storeslot);
54145436
}
@@ -5428,18 +5450,10 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
54285450
original_insert_tuple);
54295451
else if (map != NULL)
54305452
{
5453+
AfterTriggersTableData *table = transition_capture->tcs_private;
54315454
TupleTableSlot *storeslot;
54325455

5433-
storeslot = transition_capture->tcs_private->storeslot;
5434-
5435-
if (!storeslot)
5436-
{
5437-
storeslot = ExecAllocTableSlot(&estate->es_tupleTable,
5438-
map->outdesc,
5439-
&TTSOpsVirtual);
5440-
transition_capture->tcs_private->storeslot = storeslot;
5441-
}
5442-
5456+
storeslot = GetAfterTriggersStoreSlot(table, map->outdesc);
54435457
execute_attr_map_slot(map->attrMap, newslot, storeslot);
54445458
tuplestore_puttupleslot(new_tuplestore, storeslot);
54455459
}

src/test/regress/expected/triggers.out

+59
Original file line numberDiff line numberDiff line change
@@ -3201,3 +3201,62 @@ create trigger aft_row after insert or update on trigger_parted
32013201
create table trigger_parted_p1 partition of trigger_parted for values in (1)
32023202
partition by list (a);
32033203
create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
3204+
-- verify transition table conversion slot's lifetime
3205+
-- https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com
3206+
create table convslot_test_parent (col1 text primary key);
3207+
create table convslot_test_child (col1 text primary key,
3208+
foreign key (col1) references convslot_test_parent(col1) on delete cascade on update cascade
3209+
);
3210+
alter table convslot_test_child add column col2 text not null default 'tutu';
3211+
insert into convslot_test_parent(col1) values ('1');
3212+
insert into convslot_test_child(col1) values ('1');
3213+
insert into convslot_test_parent(col1) values ('3');
3214+
insert into convslot_test_child(col1) values ('3');
3215+
create or replace function trigger_function1()
3216+
returns trigger
3217+
language plpgsql
3218+
AS $$
3219+
begin
3220+
raise notice 'trigger = %, old_table = %',
3221+
TG_NAME,
3222+
(select string_agg(old_table::text, ', ' order by col1) from old_table);
3223+
return null;
3224+
end; $$;
3225+
create or replace function trigger_function2()
3226+
returns trigger
3227+
language plpgsql
3228+
AS $$
3229+
begin
3230+
raise notice 'trigger = %, new table = %',
3231+
TG_NAME,
3232+
(select string_agg(new_table::text, ', ' order by col1) from new_table);
3233+
return null;
3234+
end; $$;
3235+
create trigger but_trigger after update on convslot_test_child
3236+
referencing new table as new_table
3237+
for each statement execute function trigger_function2();
3238+
update convslot_test_parent set col1 = col1 || '1';
3239+
NOTICE: trigger = but_trigger, new table = (11,tutu), (31,tutu)
3240+
create or replace function trigger_function3()
3241+
returns trigger
3242+
language plpgsql
3243+
AS $$
3244+
begin
3245+
raise notice 'trigger = %, old_table = %, new table = %',
3246+
TG_NAME,
3247+
(select string_agg(old_table::text, ', ' order by col1) from old_table),
3248+
(select string_agg(new_table::text, ', ' order by col1) from new_table);
3249+
return null;
3250+
end; $$;
3251+
create trigger but_trigger2 after update on convslot_test_child
3252+
referencing old table as old_table new table as new_table
3253+
for each statement execute function trigger_function3();
3254+
update convslot_test_parent set col1 = col1 || '1';
3255+
NOTICE: trigger = but_trigger, new table = (111,tutu), (311,tutu)
3256+
NOTICE: trigger = but_trigger2, old_table = (11,tutu), (31,tutu), new table = (111,tutu), (311,tutu)
3257+
create trigger bdt_trigger after delete on convslot_test_child
3258+
referencing old table as old_table
3259+
for each statement execute function trigger_function1();
3260+
delete from convslot_test_parent;
3261+
NOTICE: trigger = bdt_trigger, old_table = (111,tutu), (311,tutu)
3262+
drop table convslot_test_child, convslot_test_parent;

src/test/regress/sql/triggers.sql

+65
Original file line numberDiff line numberDiff line change
@@ -2383,3 +2383,68 @@ create trigger aft_row after insert or update on trigger_parted
23832383
create table trigger_parted_p1 partition of trigger_parted for values in (1)
23842384
partition by list (a);
23852385
create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);
2386+
2387+
-- verify transition table conversion slot's lifetime
2388+
-- https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com
2389+
create table convslot_test_parent (col1 text primary key);
2390+
create table convslot_test_child (col1 text primary key,
2391+
foreign key (col1) references convslot_test_parent(col1) on delete cascade on update cascade
2392+
);
2393+
2394+
alter table convslot_test_child add column col2 text not null default 'tutu';
2395+
insert into convslot_test_parent(col1) values ('1');
2396+
insert into convslot_test_child(col1) values ('1');
2397+
insert into convslot_test_parent(col1) values ('3');
2398+
insert into convslot_test_child(col1) values ('3');
2399+
2400+
create or replace function trigger_function1()
2401+
returns trigger
2402+
language plpgsql
2403+
AS $$
2404+
begin
2405+
raise notice 'trigger = %, old_table = %',
2406+
TG_NAME,
2407+
(select string_agg(old_table::text, ', ' order by col1) from old_table);
2408+
return null;
2409+
end; $$;
2410+
2411+
create or replace function trigger_function2()
2412+
returns trigger
2413+
language plpgsql
2414+
AS $$
2415+
begin
2416+
raise notice 'trigger = %, new table = %',
2417+
TG_NAME,
2418+
(select string_agg(new_table::text, ', ' order by col1) from new_table);
2419+
return null;
2420+
end; $$;
2421+
2422+
create trigger but_trigger after update on convslot_test_child
2423+
referencing new table as new_table
2424+
for each statement execute function trigger_function2();
2425+
2426+
update convslot_test_parent set col1 = col1 || '1';
2427+
2428+
create or replace function trigger_function3()
2429+
returns trigger
2430+
language plpgsql
2431+
AS $$
2432+
begin
2433+
raise notice 'trigger = %, old_table = %, new table = %',
2434+
TG_NAME,
2435+
(select string_agg(old_table::text, ', ' order by col1) from old_table),
2436+
(select string_agg(new_table::text, ', ' order by col1) from new_table);
2437+
return null;
2438+
end; $$;
2439+
2440+
create trigger but_trigger2 after update on convslot_test_child
2441+
referencing old table as old_table new table as new_table
2442+
for each statement execute function trigger_function3();
2443+
update convslot_test_parent set col1 = col1 || '1';
2444+
2445+
create trigger bdt_trigger after delete on convslot_test_child
2446+
referencing old table as old_table
2447+
for each statement execute function trigger_function1();
2448+
delete from convslot_test_parent;
2449+
2450+
drop table convslot_test_child, convslot_test_parent;

0 commit comments

Comments
 (0)