pgsql: Prevent access to no-longer-pinned buffer in heapam_tuple_lock() - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Prevent access to no-longer-pinned buffer in heapam_tuple_lock()
Date
Msg-id E1neguY-000MdF-LM@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Prevent access to no-longer-pinned buffer in heapam_tuple_lock().

heap_fetch() used to have a "keep_buf" parameter that told it to return
ownership of the buffer pin to the caller after finding that the
requested tuple TID exists but is invisible to the specified snapshot.
This was thoughtlessly removed in commit 5db6df0c0, which broke
heapam_tuple_lock() (formerly EvalPlanQualFetch) because that function
needs to do more accesses to the tuple even if it's invisible.  The net
effect is that we would continue to touch the page for a microsecond or
two after releasing pin on the buffer.  Usually no harm would result;
but if a different session decided to defragment the page concurrently,
we could see garbage data and mistakenly conclude that there's no newer
tuple version to chain up to.  (It's hard to say whether this has
happened in the field.  The bug was actually found thanks to a later
change that allowed valgrind to detect accesses to non-pinned buffers.)

The most reasonable way to fix this is to reintroduce keep_buf,
although I made it behave slightly differently: buffer ownership
is passed back only if there is a valid tuple at the requested TID.
In HEAD, we can just add the parameter back to heap_fetch().
To avoid an API break in the back branches, introduce an additional
function heap_fetch_extended() in those branches.

In HEAD there is an additional, less obvious API change: tuple->t_data
will be set to NULL in all cases where buffer ownership is not returned,
in particular when the tuple exists but fails the time qual (and
!keep_buf).  This is to defend against any other callers attempting to
access non-pinned buffers.  We concluded that making that change in back
branches would be more likely to introduce problems than cure any.

In passing, remove a comment about heap_fetch that was obsoleted by
9a8ee1dc6.

Per bug #17462 from Daniil Anisimov.  Back-patch to v12 where the bug
was introduced.

Discussion: https://postgr.es/m/17462-9c98a0f00df9bd36@postgresql.org

Branch
------
REL_12_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/e0ed20d0b66ed2d1395e75dbb856d9b4cd251ad9

Modified Files
--------------
src/backend/access/heap/heapam.c         | 40 ++++++++++++++++++++++++++------
src/backend/access/heap/heapam_handler.c | 15 ++++++------
src/include/access/heapam.h              |  3 +++
3 files changed, 43 insertions(+), 15 deletions(-)


pgsql-committers by date:

Previous
From: Alvaro Herrera
Date:
Subject: pgsql: Remove extraneous blank lines before block-closing braces
Next
From: Tom Lane
Date:
Subject: pgsql: Further tweak the default behavior of psql's \dconfig.