Re: [PATCH] bms_prev_member() can read beyond the end of the array of allocated words

From: "Burd, Greg" <greg(at)burd(dot)me>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] bms_prev_member() can read beyond the end of the array of allocated words
Date: 2025-08-15 13:58:09
Message-ID: F4749748-F936-4B05-833F-C401B69C97A3@burd.me
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


> On Aug 15, 2025, at 12:37 AM, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> On Fri, 15 Aug 2025 at 15:24, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>
>> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
>>> I'm happy to push Greg's v5 patch if you have no counterarguments.
>>
>> In the end this isn't something I find worth arguing about. If
>> you prefer v5, sure. I do suggest though that if we're installing
>> Asserts at all, defending against prevbit < -1 is worth doing.

Agreed, that's a reasonable addition.

> Agreed about defending against prevbit < -1. I added an Assert for
> that. Technically, that Assert could be up above the if (a == NULL)
> check, but I didn't think it mattered that much and opted to keep both
> Asserts together. The difference being that bms_prev_member(NULL, -2)
> will return -2 rather than Assert fail. I'm not too worried about
> that, but if you feel strongly differently, I can adjust what I just
> pushed.

Thanks for adding that Assert. I don't think it's worth relocating at
this time.

I'll post the addition of tests in a new thread. I hope you both have
interest and time to review that as well.

> David

Thanks David for pushing the commit! :)

best.

-greg

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Sami Imseih 2025-08-15 14:17:22 shmem_startup_hook called twice on Windows
Previous Message Jingtang Zhang 2025-08-15 12:50:20 Memory leak of SMgrRelation object on standby