]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Prevent possible double-free when update trigger returns old tuple.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 16 Aug 2019 00:04:19 +0000 (20:04 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 16 Aug 2019 00:04:19 +0000 (20:04 -0400)
This is a variant of the problem fixed in commit 25b692568, which
unfortunately we failed to detect at the time.  If an update trigger
returns the "old" tuple, as it's entitled to do, then a subsequent
iteration of the loop in ExecBRUpdateTriggers would have "oldtuple"
equal to "trigtuple" and would fail to notice that it shouldn't
free that.

In addition to fixing the code, extend the test case added by
25b692568 so that it covers multiple-trigger-iterations cases.

This problem does not manifest in v12/HEAD, as a result of the
relevant code having been largely rewritten for slotification.
However, include the test case into v12/HEAD anyway, since this
is clearly an area that someone could break again in future.

Per report from Piotr Gabriel Kosinski.  Back-patch into all
supported branches, since the bug seems quite old.

Diagnosis and code fix by Thomas Munro, test case by me.

Discussion: https://postgr.es/m/CAFMLSdP0rd7LqC3j-H6Fh51FYSt5A10DDh-3=W4PPc4LLUQ8YQ@mail.gmail.com

src/backend/commands/trigger.c
src/test/regress/expected/triggers.out
src/test/regress/sql/triggers.sql

index 7887ffc68d427521bb72d77289098412111b1f04..1b10f2a37fe7c9529a334a241f715b695ad1c054 100644 (file)
@@ -2489,7 +2489,9 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
                                                                           relinfo->ri_TrigFunctions,
                                                                           relinfo->ri_TrigInstrument,
                                                                           GetPerTupleMemoryContext(estate));
-               if (oldtuple != newtuple && oldtuple != slottuple)
+               if (oldtuple != newtuple &&
+                       oldtuple != slottuple &&
+                       oldtuple != trigtuple)
                        heap_freetuple(oldtuple);
                if (newtuple == NULL)
                {
index 57bce17abb2ea5044ba1f99665f515f3788a9305..9b99fa4c527431e5ee5f5533715082e20e855abf 100644 (file)
@@ -158,6 +158,76 @@ select * from trigtest;
 ----+----
 (0 rows)
 
+-- Also check what happens when such a trigger runs before or after others
+create function f1_times_10() returns trigger as
+$$ begin new.f1 := new.f1 * 10; return new; end $$ language plpgsql;
+create trigger trigger_alpha
+       before insert or update on trigtest
+       for each row execute procedure f1_times_10();
+insert into trigtest values(1, 'foo');
+select * from trigtest;
+ f1 | f2  
+----+-----
+ 10 | foo
+(1 row)
+
+update trigtest set f2 = f2 || 'bar';
+select * from trigtest;
+ f1 | f2  
+----+-----
+ 10 | foo
+(1 row)
+
+delete from trigtest;
+select * from trigtest;
+ f1 | f2 
+----+----
+(0 rows)
+
+create trigger trigger_zed
+       before insert or update on trigtest
+       for each row execute procedure f1_times_10();
+insert into trigtest values(1, 'foo');
+select * from trigtest;
+ f1  | f2  
+-----+-----
+ 100 | foo
+(1 row)
+
+update trigtest set f2 = f2 || 'bar';
+select * from trigtest;
+  f1  | f2  
+------+-----
+ 1000 | foo
+(1 row)
+
+delete from trigtest;
+select * from trigtest;
+ f1 | f2 
+----+----
+(0 rows)
+
+drop trigger trigger_alpha on trigtest;
+insert into trigtest values(1, 'foo');
+select * from trigtest;
+ f1 | f2  
+----+-----
+ 10 | foo
+(1 row)
+
+update trigtest set f2 = f2 || 'bar';
+select * from trigtest;
+ f1  | f2  
+-----+-----
+ 100 | foo
+(1 row)
+
+delete from trigtest;
+select * from trigtest;
+ f1 | f2 
+----+----
+(0 rows)
+
 drop table trigtest;
 create sequence ttdummy_seq increment 10 start 0 minvalue 0;
 create table tttest (
index 1c05b665d121ea6ecdb4bc341c52ca91d3de9225..8a747b638d15fe67faaee996a93e4016b4c4dc90 100644 (file)
@@ -145,6 +145,41 @@ select * from trigtest;
 delete from trigtest;
 select * from trigtest;
 
+-- Also check what happens when such a trigger runs before or after others
+create function f1_times_10() returns trigger as
+$$ begin new.f1 := new.f1 * 10; return new; end $$ language plpgsql;
+
+create trigger trigger_alpha
+       before insert or update on trigtest
+       for each row execute procedure f1_times_10();
+
+insert into trigtest values(1, 'foo');
+select * from trigtest;
+update trigtest set f2 = f2 || 'bar';
+select * from trigtest;
+delete from trigtest;
+select * from trigtest;
+
+create trigger trigger_zed
+       before insert or update on trigtest
+       for each row execute procedure f1_times_10();
+
+insert into trigtest values(1, 'foo');
+select * from trigtest;
+update trigtest set f2 = f2 || 'bar';
+select * from trigtest;
+delete from trigtest;
+select * from trigtest;
+
+drop trigger trigger_alpha on trigtest;
+
+insert into trigtest values(1, 'foo');
+select * from trigtest;
+update trigtest set f2 = f2 || 'bar';
+select * from trigtest;
+delete from trigtest;
+select * from trigtest;
+
 drop table trigtest;
 
 create sequence ttdummy_seq increment 10 start 0 minvalue 0;