pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Prevent dangling-pointer access when update trigger returns old 6+ messages / 1 participants [nested] [flat]
* pgsql: Prevent dangling-pointer access when update trigger returns old @ 2018-02-27 18:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2018-02-27 18:28 UTC (permalink / raw) To: pgsql-committers Prevent dangling-pointer access when update trigger returns old tuple. A before-update row trigger may choose to return the "new" or "old" tuple unmodified. ExecBRUpdateTriggers failed to consider the second possibility, and would proceed to free the "old" tuple even if it was the one returned, leading to subsequent access to already-deallocated memory. In debug builds this reliably leads to an "invalid memory alloc request size" failure; in production builds it might accidentally work, but data corruption is also possible. This is a very old bug. There are probably a couple of reasons it hasn't been noticed up to now. It would be more usual to return NULL if one wanted to suppress the update action; returning "old" is significantly less efficient since the update will occur anyway. Also, none of the standard PLs would ever cause this because they all returned freshly-manufactured tuples even if they were just copying "old". But commit 4b93f5799 changed that 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's ExecBRUpdateTriggers's fault not plpgsql's. It seems worth creating a test case that exercises returning "old" directly with a C-language trigger; testing this through plpgsql seems unreliable because 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 Branch ------ REL9_3_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/9bc33ef5ece99faec588b93e5ebda0448ab2ece5 Modified Files -------------- src/backend/commands/trigger.c | 3 ++- src/test/regress/expected/triggers.out | 26 ++++++++++++++++++++++++ src/test/regress/input/create_function_1.source | 5 +++++ src/test/regress/output/create_function_1.source | 4 ++++ src/test/regress/regress.c | 18 ++++++++++++++++ src/test/regress/sql/triggers.sql | 16 +++++++++++++++ 6 files changed, 71 insertions(+), 1 deletion(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Prevent dangling-pointer access when update trigger returns old @ 2018-02-27 18:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2018-02-27 18:28 UTC (permalink / raw) To: pgsql-committers Prevent dangling-pointer access when update trigger returns old tuple. A before-update row trigger may choose to return the "new" or "old" tuple unmodified. ExecBRUpdateTriggers failed to consider the second possibility, and would proceed to free the "old" tuple even if it was the one returned, leading to subsequent access to already-deallocated memory. In debug builds this reliably leads to an "invalid memory alloc request size" failure; in production builds it might accidentally work, but data corruption is also possible. This is a very old bug. There are probably a couple of reasons it hasn't been noticed up to now. It would be more usual to return NULL if one wanted to suppress the update action; returning "old" is significantly less efficient since the update will occur anyway. Also, none of the standard PLs would ever cause this because they all returned freshly-manufactured tuples even if they were just copying "old". But commit 4b93f5799 changed that 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's ExecBRUpdateTriggers's fault not plpgsql's. It seems worth creating a test case that exercises returning "old" directly with a C-language trigger; testing this through plpgsql seems unreliable because 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 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/25b692568f429436f89ff203c1413e9670d0ad67 Modified Files -------------- src/backend/commands/trigger.c | 2 +- src/test/regress/expected/triggers.out | 26 ++++++++++++++++++++++++ src/test/regress/input/create_function_1.source | 5 +++++ src/test/regress/output/create_function_1.source | 4 ++++ src/test/regress/regress.c | 15 ++++++++++++++ src/test/regress/sql/triggers.sql | 16 +++++++++++++++ 6 files changed, 67 insertions(+), 1 deletion(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Prevent dangling-pointer access when update trigger returns old @ 2018-02-27 18:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2018-02-27 18:28 UTC (permalink / raw) To: pgsql-committers Prevent dangling-pointer access when update trigger returns old tuple. A before-update row trigger may choose to return the "new" or "old" tuple unmodified. ExecBRUpdateTriggers failed to consider the second possibility, and would proceed to free the "old" tuple even if it was the one returned, leading to subsequent access to already-deallocated memory. In debug builds this reliably leads to an "invalid memory alloc request size" failure; in production builds it might accidentally work, but data corruption is also possible. This is a very old bug. There are probably a couple of reasons it hasn't been noticed up to now. It would be more usual to return NULL if one wanted to suppress the update action; returning "old" is significantly less efficient since the update will occur anyway. Also, none of the standard PLs would ever cause this because they all returned freshly-manufactured tuples even if they were just copying "old". But commit 4b93f5799 changed that 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's ExecBRUpdateTriggers's fault not plpgsql's. It seems worth creating a test case that exercises returning "old" directly with a C-language trigger; testing this through plpgsql seems unreliable because 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 Branch ------ REL9_5_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/2ee44e10d44b9c8c331040eac76f2769bc80865e Modified Files -------------- src/backend/commands/trigger.c | 2 +- src/test/regress/expected/triggers.out | 26 ++++++++++++++++++++++++ src/test/regress/input/create_function_1.source | 5 +++++ src/test/regress/output/create_function_1.source | 4 ++++ src/test/regress/regress.c | 16 +++++++++++++++ src/test/regress/sql/triggers.sql | 16 +++++++++++++++ 6 files changed, 68 insertions(+), 1 deletion(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Prevent dangling-pointer access when update trigger returns old @ 2018-02-27 18:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2018-02-27 18:28 UTC (permalink / raw) To: pgsql-committers Prevent dangling-pointer access when update trigger returns old tuple. A before-update row trigger may choose to return the "new" or "old" tuple unmodified. ExecBRUpdateTriggers failed to consider the second possibility, and would proceed to free the "old" tuple even if it was the one returned, leading to subsequent access to already-deallocated memory. In debug builds this reliably leads to an "invalid memory alloc request size" failure; in production builds it might accidentally work, but data corruption is also possible. This is a very old bug. There are probably a couple of reasons it hasn't been noticed up to now. It would be more usual to return NULL if one wanted to suppress the update action; returning "old" is significantly less efficient since the update will occur anyway. Also, none of the standard PLs would ever cause this because they all returned freshly-manufactured tuples even if they were just copying "old". But commit 4b93f5799 changed that 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's ExecBRUpdateTriggers's fault not plpgsql's. It seems worth creating a test case that exercises returning "old" directly with a C-language trigger; testing this through plpgsql seems unreliable because 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 Branch ------ REL9_4_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/5ccb77586955375167917c9910c0fc29cd3cfea5 Modified Files -------------- src/backend/commands/trigger.c | 2 +- src/test/regress/expected/triggers.out | 26 ++++++++++++++++++++++++ src/test/regress/input/create_function_1.source | 5 +++++ src/test/regress/output/create_function_1.source | 4 ++++ src/test/regress/regress.c | 16 +++++++++++++++ src/test/regress/sql/triggers.sql | 16 +++++++++++++++ 6 files changed, 68 insertions(+), 1 deletion(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Prevent dangling-pointer access when update trigger returns old @ 2018-02-27 18:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2018-02-27 18:28 UTC (permalink / raw) To: pgsql-committers Prevent dangling-pointer access when update trigger returns old tuple. A before-update row trigger may choose to return the "new" or "old" tuple unmodified. ExecBRUpdateTriggers failed to consider the second possibility, and would proceed to free the "old" tuple even if it was the one returned, leading to subsequent access to already-deallocated memory. In debug builds this reliably leads to an "invalid memory alloc request size" failure; in production builds it might accidentally work, but data corruption is also possible. This is a very old bug. There are probably a couple of reasons it hasn't been noticed up to now. It would be more usual to return NULL if one wanted to suppress the update action; returning "old" is significantly less efficient since the update will occur anyway. Also, none of the standard PLs would ever cause this because they all returned freshly-manufactured tuples even if they were just copying "old". But commit 4b93f5799 changed that 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's ExecBRUpdateTriggers's fault not plpgsql's. It seems worth creating a test case that exercises returning "old" directly with a C-language trigger; testing this through plpgsql seems unreliable because 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 Branch ------ REL_10_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/b45f821e2226bd10d8a6e2e601bbb1fe17c5686f Modified Files -------------- src/backend/commands/trigger.c | 2 +- src/test/regress/expected/triggers.out | 26 ++++++++++++++++++++++++ src/test/regress/input/create_function_1.source | 5 +++++ src/test/regress/output/create_function_1.source | 4 ++++ src/test/regress/regress.c | 16 +++++++++++++++ src/test/regress/sql/triggers.sql | 16 +++++++++++++++ 6 files changed, 68 insertions(+), 1 deletion(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Prevent dangling-pointer access when update trigger returns old @ 2018-02-27 18:28 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2018-02-27 18:28 UTC (permalink / raw) To: pgsql-committers Prevent dangling-pointer access when update trigger returns old tuple. A before-update row trigger may choose to return the "new" or "old" tuple unmodified. ExecBRUpdateTriggers failed to consider the second possibility, and would proceed to free the "old" tuple even if it was the one returned, leading to subsequent access to already-deallocated memory. In debug builds this reliably leads to an "invalid memory alloc request size" failure; in production builds it might accidentally work, but data corruption is also possible. This is a very old bug. There are probably a couple of reasons it hasn't been noticed up to now. It would be more usual to return NULL if one wanted to suppress the update action; returning "old" is significantly less efficient since the update will occur anyway. Also, none of the standard PLs would ever cause this because they all returned freshly-manufactured tuples even if they were just copying "old". But commit 4b93f5799 changed that 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's ExecBRUpdateTriggers's fault not plpgsql's. It seems worth creating a test case that exercises returning "old" directly with a C-language trigger; testing this through plpgsql seems unreliable because 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 Branch ------ REL9_6_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/06f47297e2b05971e89dafca01ed00f12313ca16 Modified Files -------------- src/backend/commands/trigger.c | 2 +- src/test/regress/expected/triggers.out | 26 ++++++++++++++++++++++++ src/test/regress/input/create_function_1.source | 5 +++++ src/test/regress/output/create_function_1.source | 4 ++++ src/test/regress/regress.c | 16 +++++++++++++++ src/test/regress/sql/triggers.sql | 16 +++++++++++++++ 6 files changed, 68 insertions(+), 1 deletion(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2018-02-27 18:28 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2018-02-27 18:28 pgsql: Prevent dangling-pointer access when update trigger returns old Tom Lane <tgl@sss.pgh.pa.us> 2018-02-27 18:28 pgsql: Prevent dangling-pointer access when update trigger returns old Tom Lane <tgl@sss.pgh.pa.us> 2018-02-27 18:28 pgsql: Prevent dangling-pointer access when update trigger returns old Tom Lane <tgl@sss.pgh.pa.us> 2018-02-27 18:28 pgsql: Prevent dangling-pointer access when update trigger returns old Tom Lane <tgl@sss.pgh.pa.us> 2018-02-27 18:28 pgsql: Prevent dangling-pointer access when update trigger returns old Tom Lane <tgl@sss.pgh.pa.us> 2018-02-27 18:28 pgsql: Prevent dangling-pointer access when update trigger returns old Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox