Hi Andres, 82467f627bd478569de04f4a3f1993098e80c812 added MarkBufferDirtyHint() which invokes GetBufferDescriptor() even for local buffers for which id < 0. Since GetBufferDescriptor() declares id as uint32, -1 is converted to a very large int32 value which is way larger than NBuffers. Thus GetBufferDescriptor() may be returning something from the BufferBlocks which probably has enough memory to accommodate that memory access. But it's a bogus BufferDesc nevertheless. We are not seeing any problem with this right now since MarkBufferDirtyHint() uses the BufferDesc only when it's a shared buffer. Right fix is to let that function handle local buffers first and then call GetBufferDescriptor() as in the attached patch.
I caught this because of an Assertion added in GetBufferDescription() in my shared buffer resizing patches. I think it's worth committing that assertion and the related change to BufferManagerShmemInit() separately from shared buffer resizing patches. Included those changes in the attached patch as well. -- Best Wishes, Ashutosh Bapat
From f66dd8d526451780775aa2a91de7f1781adbbc73 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Sat, 6 Jun 2026 13:21:50 +0530 Subject: [PATCH v20260606 1/6] MarkBufferDirtyHint() calls GetBufferDescriptor() for local buffers 82467f627bd478569de04f4a3f1993098e80c812 added MarkBufferDirtyHint() which invokes GetBufferDescriptor() even for local buffers which have negative buffer identifiers. Since GetBufferDescriptor() declares id as uint32, a negative integer passed to it is converted to a 32 bit integer value which is way larger than NBuffers. Thus GetBufferDescriptor() may be returning a pointer inside BufferBlocks which probably has enough memory to accommodate that memory access. But it's a bogus BufferDesc nevertheless. We are not seeing any problem with this right now since MarkBufferDirtyHint() uses the returned BufferDesc only when it is a shared buffer. Fix it by letting MarkBufferDirtyHint() handle local buffers first and then calling GetBufferDescriptor(). In order to catch bogus BufferDescs returned by GetBufferDescriptor(), add an assertion in GetBufferDescription() to check that the buffer identifier in the BufferDesc is indeed the expeted one. Because of this assertion we can not rely on using GetBufferDescriptor() in the initialization code. Hence change BufferManagerShmemInit() to access BufferDescriptors directly. Reported by: Ashutosh Bapat <[email protected]> Author: Ashutosh Bapat <[email protected]> --- src/backend/storage/buffer/buf_init.c | 3 ++- src/backend/storage/buffer/bufmgr.c | 4 ++-- src/include/storage/buf_internals.h | 6 +++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/backend/storage/buffer/buf_init.c b/src/backend/storage/buffer/buf_init.c index 1407c930c56..5b5703e1012 100644 --- a/src/backend/storage/buffer/buf_init.c +++ b/src/backend/storage/buffer/buf_init.c @@ -124,7 +124,8 @@ BufferManagerShmemInit(void *arg) */ for (int i = 0; i < NBuffers; i++) { - BufferDesc *buf = GetBufferDescriptor(i); + /* GetBufferDescriptor() expects buf_id to be set in the descriptor. */ + BufferDesc *buf = &(BufferDescriptors[i]).bufferdesc; ClearBufferTag(&buf->tag); diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index cc398db124d..d6c0cc1f6d4 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -5831,8 +5831,6 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) { BufferDesc *bufHdr; - bufHdr = GetBufferDescriptor(buffer - 1); - if (!BufferIsValid(buffer)) elog(ERROR, "bad buffer ID: %d", buffer); @@ -5842,6 +5840,8 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) return; } + bufHdr = GetBufferDescriptor(buffer - 1); + MarkSharedBufferDirtyHint(buffer, bufHdr, pg_atomic_read_u64(&bufHdr->state), buffer_std); diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index 89615a254a3..100b5272a7a 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -421,7 +421,11 @@ extern PGDLLIMPORT BufferDesc *LocalBufferDescriptors; static inline BufferDesc * GetBufferDescriptor(uint32 id) { - return &(BufferDescriptors[id]).bufferdesc; + BufferDesc *bdesc = &(BufferDescriptors[id]).bufferdesc; + + Assert(bdesc->buf_id == id); + + return bdesc; } static inline BufferDesc * base-commit: 89eafad297a9b01ad77cfc1ab93a433e0af894b0 -- 2.34.1
