upstream/postgresql
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-04-15 22:10:45 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fix unlikely overflow bug in bms_next_member()

Browse source 
... and bms_prev_member().

Both of these functions won't work correctly when given a prevbit of
INT_MAX and would crash when operating on a Bitmapset that happened to
have a member with that value.

Here we fix that by using an unsigned int to calculate which member to
look for next.

I've also adjusted bms_prev_member() to check for < 0 rather than == -1
for starting the loop.  This was done as it's safer and comes at zero
extra cost.

With our current use cases, it's likely impossible to have a Bitmapset
with an INT_MAX member, so no backpatch here.  I only noticed this issue
when working on a bms function to bitshift a Bitmapset.

Author: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CAApHDvr1B2gbf6JF69QmueM2QNRvbQeeKLxDnF=w9f9--022uA@mail.gmail.com
This commit is contained in:
David Rowley 2026-04-13 11:39:15 +12:00
parent a63bbc811d
commit e3e26d04bd
1 changed files with 20 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

34
src/backend/nodes/bitmapset.c
View file

@ -1289,6 +1289,7 @@ bms_join(Bitmapset *a, Bitmapset *b)
int
bms_next_member(const Bitmapset *a, int prevbit)
{
unsigned int currbit = prevbit;
int nwords;
bitmapword mask;
@ -1297,13 +1298,15 @@ bms_next_member(const Bitmapset *a, int prevbit)
if (a == NULL)
return -2;
nwords = a->nwords;
prevbit++;
mask = (~(bitmapword) 0) << BITNUM(prevbit);
for (int wordnum = WORDNUM(prevbit); wordnum < nwords; wordnum++)
/* use an unsigned int to avoid the risk that int overflows */
currbit++;
mask = (~(bitmapword) 0) << BITNUM(currbit);
for (int wordnum = WORDNUM(currbit); wordnum < nwords; wordnum++)
{
bitmapword w = a->words[wordnum];
/* ignore bits before prevbit */
/* ignore bits before currbit */
w &= mask;
if (w != 0)
@ -1345,10 +1348,10 @@ bms_next_member(const Bitmapset *a, int prevbit)
* It makes no difference in simple loop usage, but complex iteration logic
* might need such an ability.
*/
int
bms_prev_member(const Bitmapset *a, int prevbit)
{
unsigned int currbit;
int ushiftbits;
bitmapword mask;
@ -1362,22 +1365,25 @@ bms_prev_member(const Bitmapset *a, int prevbit)
return -2;
/* Validate callers didn't give us something out of range */
Assert(prevbit <= a->nwords * BITS_PER_BITMAPWORD);
Assert(prevbit >= -1);
Assert(prevbit < 0 || prevbit <= (unsigned int) (a->nwords * BITS_PER_BITMAPWORD));
/* transform -1 to the highest possible bit we could have set */
if (prevbit == -1)
prevbit = a->nwords * BITS_PER_BITMAPWORD - 1;
/*
* Transform -1 (or any negative number) to the highest possible bit we
* could have set. We do this in unsigned math to avoid the risk of
* overflowing a signed int.
*/
if (prevbit < 0)
currbit = (unsigned int) a->nwords * BITS_PER_BITMAPWORD - 1;
else
prevbit--;
currbit = prevbit - 1;
ushiftbits = BITS_PER_BITMAPWORD - (BITNUM(prevbit) + 1);
ushiftbits = BITS_PER_BITMAPWORD - (BITNUM(currbit) + 1);
mask = (~(bitmapword) 0) >> ushiftbits;
for (int wordnum = WORDNUM(prevbit); wordnum >= 0; wordnum--)
for (int wordnum = WORDNUM(currbit); wordnum >= 0; wordnum--)
{
bitmapword w = a->words[wordnum];
/* mask out bits left of prevbit */
/* mask out bits left of currbit */
w &= mask;
if (w != 0)