From d1c644ba4de05624fc5a63b3d5f4026c71ed4aa5 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Sat, 4 Apr 2026 16:41:23 +1300 Subject: [PATCH v2] 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. Here we fix that by using an unsigned int to calculate which member to look for next. In practise, Bitmapsets will never have such a large member, so no backpatch. Author: David Rowley Reviewed-by: Chao Li Discussion: https://postgr.es/m/CAApHDvq0T%3DiJ0Sf5TNE9yyWwfOeVjmrBt0wSywDnGD9Y4YJQBA%40mail.gmail.com --- src/backend/nodes/bitmapset.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c index 786f343b3c9..957172648c3 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,24 @@ 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 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) -- 2.51.0