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

Commit06f4729

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 parentad5fe2d commit06f4729

File tree

6 files changed

+68
-1
lines changed

6 files changed

+68
-1
lines changed

‎src/backend/commands/trigger.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2497,7 +2497,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
24972497
returnNULL;/* "do nothing" */
24982498
}
24992499
}
2500-
if (trigtuple!=fdw_trigtuple)
2500+
if (trigtuple!=fdw_trigtuple&&trigtuple!=newtuple)
25012501
heap_freetuple(trigtuple);
25022502

25032503
if (newtuple!=slottuple)

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,32 @@ DROP TABLE fkeys2;
133133
-- select count(*) from dup17 where x = 13;
134134
--
135135
-- DROP TABLE dup17;
136+
-- Check behavior when trigger returns unmodified trigtuple
137+
create table trigtest (f1 int, f2 text);
138+
create trigger trigger_return_old
139+
before insert or delete or update on trigtest
140+
for each row execute procedure trigger_return_old();
141+
insert into trigtest values(1, 'foo');
142+
select * from trigtest;
143+
f1 | f2
144+
----+-----
145+
1 | foo
146+
(1 row)
147+
148+
update trigtest set f2 = f2 || 'bar';
149+
select * from trigtest;
150+
f1 | f2
151+
----+-----
152+
1 | foo
153+
(1 row)
154+
155+
delete from trigtest;
156+
select * from trigtest;
157+
f1 | f2
158+
----+----
159+
(0 rows)
160+
161+
drop table trigtest;
136162
create sequence ttdummy_seq increment 10 start 0 minvalue 0;
137163
create table tttest (
138164
price_idint4,

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ CREATE FUNCTION funny_dup17 ()
4242
AS '@libdir@/regress@DLSUFFIX@'
4343
LANGUAGE C;
4444

45+
CREATE FUNCTION trigger_return_old ()
46+
RETURNS trigger
47+
AS '@libdir@/regress@DLSUFFIX@'
48+
LANGUAGE C;
49+
4550
CREATE FUNCTION ttdummy ()
4651
RETURNS trigger
4752
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
@@ -39,6 +39,10 @@ CREATE FUNCTION funny_dup17 ()
3939
RETURNS trigger
4040
AS '@libdir@/regress@DLSUFFIX@'
4141
LANGUAGE C;
42+
CREATE FUNCTION trigger_return_old ()
43+
RETURNS trigger
44+
AS '@libdir@/regress@DLSUFFIX@'
45+
LANGUAGE C;
4246
CREATE FUNCTION ttdummy ()
4347
RETURNS trigger
4448
AS '@libdir@/regress@DLSUFFIX@'

‎src/test/regress/regress.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,22 @@ funny_dup17(PG_FUNCTION_ARGS)
456456
returnPointerGetDatum(tuple);
457457
}
458458

459+
PG_FUNCTION_INFO_V1(trigger_return_old);
460+
461+
Datum
462+
trigger_return_old(PG_FUNCTION_ARGS)
463+
{
464+
TriggerData*trigdata= (TriggerData*)fcinfo->context;
465+
HeapTupletuple;
466+
467+
if (!CALLED_AS_TRIGGER(fcinfo))
468+
elog(ERROR,"trigger_return_old: not fired by trigger manager");
469+
470+
tuple=trigdata->tg_trigtuple;
471+
472+
returnPointerGetDatum(tuple);
473+
}
474+
459475
#defineTTDUMMY_INFINITY999999
460476

461477
staticSPIPlanPtrsplan=NULL;

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,22 @@ DROP TABLE fkeys2;
131131
--
132132
-- DROP TABLE dup17;
133133

134+
-- Check behavior when trigger returns unmodified trigtuple
135+
createtabletrigtest (f1int, f2text);
136+
137+
createtriggertrigger_return_old
138+
before insertordeleteorupdateon trigtest
139+
for each row execute procedure trigger_return_old();
140+
141+
insert into trigtestvalues(1,'foo');
142+
select*from trigtest;
143+
update trigtestset f2= f2||'bar';
144+
select*from trigtest;
145+
deletefrom trigtest;
146+
select*from trigtest;
147+
148+
droptable trigtest;
149+
134150
createsequencettdummy_seq increment10 start0 minvalue0;
135151

136152
createtabletttest (

0 commit comments

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