Fix ReadRecentBuffer for local buffers.

It incorrectly used GetBufferDescriptor instead of
GetLocalBufferDescriptor, causing it to not find the correct buffer in
most cases, and performing an out-of-bounds memory read in the corner
case that temp_buffers > shared_buffers.

It also bumped the usage-count on the buffer, even if it was
previously pinned. That won't lead to crashes or incorrect results,
but it's different from what the shared-buffer case does, and
different from the usual code in LocalBufferAlloc. Fix that too, and
make the code ordering match LocalBufferAlloc() more closely, so that
it's easier to verify that it's doing the same thing.

Currently, ReadRecentBuffer() is only used with non-temp relations, in
WAL redo, so the broken code is currently dead code. However, it could
be used by extensions.

Backpatch-through: 14
Discussion: https://www.postgresql.org/message-id/2d74b46f-27c9-fb31-7f99-327a87184cc0%40iki.fi
Reviewed-by: Thomas Munro, Zhang Mingli, Richard Guo

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index c7d7abcd738184461f13d8859ff53e1b423adca9..b7488b5d89e73ab218889e62ea9d055dc3c2e224 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -636,18 +636,28 @@ ReadRecentBuffer(RelFileLocator rlocator, ForkNumber forkNum, BlockNumber blockN
 
        if (BufferIsLocal(recent_buffer))
        {
-               bufHdr = GetBufferDescriptor(-recent_buffer - 1);
+               int                     b = -recent_buffer - 1;
+
+               bufHdr = GetLocalBufferDescriptor(b);
                buf_state = pg_atomic_read_u32(&bufHdr->state);
 
                /* Is it still valid and holding the right tag? */
                if ((buf_state & BM_VALID) && BUFFERTAGS_EQUAL(tag, bufHdr->tag))
                {
-                       /* Bump local buffer's ref and usage counts. */
+                       /*
+                        * Bump buffer's ref and usage counts. This is equivalent of
+                        * PinBuffer for a shared buffer.
+                        */
+                       if (LocalRefCount[b] == 0)
+                       {
+                               if (BUF_STATE_GET_USAGECOUNT(buf_state) < BM_MAX_USAGE_COUNT)
+                               {
+                                       buf_state += BUF_USAGECOUNT_ONE;
+                                       pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
+                               }
+                       }
+                       LocalRefCount[b]++;
                        ResourceOwnerRememberBuffer(CurrentResourceOwner, recent_buffer);
-                       LocalRefCount[-recent_buffer - 1]++;
-                       if (BUF_STATE_GET_USAGECOUNT(buf_state) < BM_MAX_USAGE_COUNT)
-                               pg_atomic_write_u32(&bufHdr->state,
-                                                                       buf_state + BUF_USAGECOUNT_ONE);
 
                        pgBufferUsage.local_blks_hit++;