Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit25b6925

Browse files
committed
Prevent dangling-pointer access when update trigger returns old tuple.
A before-update row trigger may choose to return the "new" or "old" tupleunmodified. ExecBRUpdateTriggers failed to consider the secondpossibility, and would proceed to free the "old" tuple even if it was theone returned, leading to subsequent access to already-deallocated memory.In debug builds this reliably leads to an "invalid memory alloc requestsize" failure; in production builds it might accidentally work, but datacorruption is also possible.This is a very old bug. There are probably a couple of reasons it hasn'tbeen noticed up to now. It would be more usual to return NULL if onewanted to suppress the update action; returning "old" is significantly lessefficient since the update will occur anyway. Also, none of the standardPLs would ever cause this because they all returned freshly-manufacturedtuples even if they were just copying "old". But commit4b93f57 changedthat for plpgsql, making it possible to see the bug with a plpgsql trigger.Still, this is certainly legal behavior for a trigger function, so it'sExecBRUpdateTriggers's fault not plpgsql's.It seems worth creating a test case that exercises returning "old" directlywith a C-language trigger; testing this through plpgsql seems unreliablebecause its behavior might change again.Report and fix by Rushabh Lathia; regression test case by me.Back-patch to all supported branches.Discussion:https://postgr.es/m/CAGPqQf1P4pjiNPrMof=P_16E-DFjt457j+nH2ex3=nBTew7tXw@mail.gmail.com
1 parent5e6a63c commit25b6925

File tree

6 files changed

+67
-1
lines changed

6 files changed

+67
-1
lines changed

‎src/backend/commands/trigger.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2815,7 +2815,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
28152815
returnNULL;/* "do nothing" */
28162816
}
28172817
}
2818-
if (trigtuple!=fdw_trigtuple)
2818+
if (trigtuple!=fdw_trigtuple&&trigtuple!=newtuple)
28192819
heap_freetuple(trigtuple);
28202820

28212821
if (newtuple!=slottuple)

‎src/test/regress/expected/triggers.out

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,32 @@ SELECT trigger_name, event_manipulation, event_object_schema, event_object_table
119119
DROP TABLE pkeys;
120120
DROP TABLE fkeys;
121121
DROP TABLE fkeys2;
122+
-- Check behavior when trigger returns unmodified trigtuple
123+
create table trigtest (f1 int, f2 text);
124+
create trigger trigger_return_old
125+
before insert or delete or update on trigtest
126+
for each row execute procedure trigger_return_old();
127+
insert into trigtest values(1, 'foo');
128+
select * from trigtest;
129+
f1 | f2
130+
----+-----
131+
1 | foo
132+
(1 row)
133+
134+
update trigtest set f2 = f2 || 'bar';
135+
select * from trigtest;
136+
f1 | f2
137+
----+-----
138+
1 | foo
139+
(1 row)
140+
141+
delete from trigtest;
142+
select * from trigtest;
143+
f1 | f2
144+
----+----
145+
(0 rows)
146+
147+
drop table trigtest;
122148
create sequence ttdummy_seq increment 10 start 0 minvalue 0;
123149
create table tttest (
124150
price_idint4,

‎src/test/regress/input/create_function_1.source

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ CREATE FUNCTION autoinc ()
3737
AS '@libdir@/autoinc@DLSUFFIX@'
3838
LANGUAGE C;
3939

40+
CREATE FUNCTION trigger_return_old ()
41+
RETURNS trigger
42+
AS '@libdir@/regress@DLSUFFIX@'
43+
LANGUAGE C;
44+
4045
CREATE FUNCTION ttdummy ()
4146
RETURNS trigger
4247
AS '@libdir@/regress@DLSUFFIX@'

‎src/test/regress/output/create_function_1.source

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ CREATE FUNCTION autoinc ()
3535
RETURNS trigger
3636
AS '@libdir@/autoinc@DLSUFFIX@'
3737
LANGUAGE C;
38+
CREATE FUNCTION trigger_return_old ()
39+
RETURNS trigger
40+
AS '@libdir@/regress@DLSUFFIX@'
41+
LANGUAGE C;
3842
CREATE FUNCTION ttdummy ()
3943
RETURNS trigger
4044
AS '@libdir@/regress@DLSUFFIX@'

‎src/test/regress/regress.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,21 @@ reverse_name(PG_FUNCTION_ARGS)
203203
PG_RETURN_CSTRING(new_string);
204204
}
205205

206+
PG_FUNCTION_INFO_V1(trigger_return_old);
207+
208+
Datum
209+
trigger_return_old(PG_FUNCTION_ARGS)
210+
{
211+
TriggerData*trigdata= (TriggerData*)fcinfo->context;
212+
HeapTupletuple;
213+
214+
if (!CALLED_AS_TRIGGER(fcinfo))
215+
elog(ERROR,"trigger_return_old: not fired by trigger manager");
216+
217+
tuple=trigdata->tg_trigtuple;
218+
219+
returnPointerGetDatum(tuple);
220+
}
206221

207222
#defineTTDUMMY_INFINITY999999
208223

‎src/test/regress/sql/triggers.sql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,22 @@ DROP TABLE pkeys;
103103
DROPTABLE fkeys;
104104
DROPTABLE fkeys2;
105105

106+
-- Check behavior when trigger returns unmodified trigtuple
107+
createtabletrigtest (f1int, f2text);
108+
109+
createtriggertrigger_return_old
110+
before insertordeleteorupdateon trigtest
111+
for each row execute procedure trigger_return_old();
112+
113+
insert into trigtestvalues(1,'foo');
114+
select*from trigtest;
115+
update trigtestset f2= f2||'bar';
116+
select*from trigtest;
117+
deletefrom trigtest;
118+
select*from trigtest;
119+
120+
droptable trigtest;
121+
106122
createsequencettdummy_seq increment10 start0 minvalue0;
107123

108124
createtabletttest (

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp