• Tom Lane's avatar
    Prevent access to no-longer-pinned buffer in heapam_tuple_lock(). · c590e514
    Tom Lane authored
    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 5db6df0c, 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
    9a8ee1dc.
    
    Per bug #17462 from Daniil Anisimov.  Back-patch to v12 where the bug
    was introduced.
    
    Discussion: https://postgr.es/m/17462-9c98a0f00df9bd36@postgresql.org
    c590e514
heapam.c 301 KB