pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-committers@postgresql.org
Subject: pgsql: Prevent dangling-pointer access when update trigger returns old
Date: Tue, 27 Feb 2018 18:28:16 +0000
Message-ID: <E1eqjz6-00043z-7g@gemulon.postgresql.org> (raw)

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(-)



view thread (6+ messages)  latest in thread

Message-ID: <E1eqjz6-00043z-7g@gemulon.postgresql.org>
Permalink:  ../E1eqjz6-00043z-7g@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1eqjz6-00043z-7g@gemulon.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: tgl@sss.pgh.pa.us
  Subject: Re: pgsql: Prevent dangling-pointer access when update trigger returns old
  In-Reply-To: <E1eqjz6-00043z-7g@gemulon.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

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