Re: Unsafe access BufferDescriptors array in BufferGetLSNAtomic()

From: Richard Guo <guofenglinux(at)gmail(dot)com>
To: Tender Wang <tndrwang(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Unsafe access BufferDescriptors array in BufferGetLSNAtomic()
Date: 2025-02-04 07:59:43
Message-ID: CAMbWs4-TDKBimTiRYCOxKMyHw2e7+qAQHnBLOOcLTKtKUK9-TA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Nov 7, 2024 at 7:07 PM Tender Wang <tndrwang(at)gmail(dot)com> wrote:
> While learning gist index insert codes, I find a little issue with BufferGetLSNAtomic().
> At first, it wants to get bufHdr by accessing the buffer descriptor array, as below:
>
> BufferDesc *bufHdr = GetBufferDescriptor(buffer - 1);
>
> However, it doesn't check whether the passed buffer is a local or shared buffer.
> If the buffer is local, then buffer < 0; it will be cast to uint32 when
> passed to GetBufferDescriptor().
> This may be unsafe, although no someone reports the problem.
>
> I tweak a few codes; see the attached patch.
> Any thoughts?

LGTM. When considering a local buffer, the GetBufferDescriptor() call
in BufferGetLSNAtomic() would be retrieving a shared buffer with a bad
buffer ID. Since the code checks whether the buffer is shared before
using the retrieved BufferDesc, this issue did not lead to any
malfunction. Nonetheless this seems like trouble waiting to happen.

Thanks
Richard

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2025-02-04 08:15:29 Re: POC: enable logical decoding when wal_level = 'replica' without a server restart
Previous Message Michael Paquier 2025-02-04 07:55:03 Re: Show WAL write and fsync stats in pg_stat_io