Re: [PATCH] bms_prev_member() can read beyond the end of the array of allocated words - Mailing list pgsql-hackers

From Greg Burd
Subject Re: [PATCH] bms_prev_member() can read beyond the end of the array of allocated words
Date
Msg-id 705A1F87-1502-4D3C-B24B-4F7E72C8B584@getmailspring.com
Whole thread Raw
In response to Re: [PATCH] bms_prev_member() can read beyond the end of the array of allocated words  (Greg Burd <greg@burd.me>)
Responses Re: [PATCH] bms_prev_member() can read beyond the end of the array of allocated words
Re: [PATCH] bms_prev_member() can read beyond the end of the array of allocated words
List pgsql-hackers
On Aug 14 2025, at 11:37 am, Greg Burd <greg@burd.me> wrote:

> 
> On Aug 14 2025, at 11:14 am, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
>> David Rowley <dgrowleyml@gmail.com> writes:
>>> It is valid to pass prevbit as a->nwords * BITS_PER_BITMAPWORD as the
>>> code does "prevbit--;". Maybe it would be less confusing if it were
>>> written as:
>>>  * "prevbit" must be less than or equal to "a->nwords * BITS_PER_BITMAPWORD".
>>> The Assert should be using <= rather than <.
>> 
>> Actually, I don't agree with that.  It's true that it wouldn't fail,
>> but a caller doing that is exhibiting undue intimacy with the innards
>> of Bitmapsets.  The expected usage is that the argument is initially
>> -1 and after that the result of the previous call (which'll
>> necessarily be less than a->nwords * BITS_PER_BITMAPWORD).  We don't
>> have any state with which we can verify the chain of calls, but it
>> seems totally reasonable to me to disallow an outside caller
>> providing an argument >= a->nwords * BITS_PER_BITMAPWORD.
>> 
>>             regards, tom lane
> 
> 
> Thanks Tom, David,
> 
> Seems I also forgot about the case where the Bitmapset passed is NULL. 
> The new assert needs to handle that as well.
> 
> -greg

Well, that was rushed.  Apologies.

-greg

Attachment

pgsql-hackers by date:

Previous
From: Benoît Dufour
Date:
Subject: [Feature request] Add a way to get the length of a PQerrorMessage in libpq
Next
From: Tom Lane
Date:
Subject: Re: [PATCH] Silence Valgrind about SelectConfigFiles()