From e3e26d04bd52795083b3947151c5c71e989a01f8 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Mon, 13 Apr 2026 11:39:15 +1200 Subject: [PATCH] Fix unlikely overflow bug in bms_next_member() ... 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 Reviewed-by: Chao Li Discussion: https://postgr.es/m/CAApHDvr1B2gbf6JF69QmueM2QNRvbQeeKLxDnF=w9f9--022uA@mail.gmail.com --- src/backend/nodes/bitmapset.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c index 786f343b3c9..f053d8c4d64 100644 --- a/src/backend/nodes/bitmapset.c +++ b/src/backend/nodes/bitmapset.c @@ -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)