Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: Small and unlikely overflow hazard in bms_next_member()
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CAApHDvq0T=iJ0Sf5TNE9yyWwfOeVjmrBt0wSywDnGD9Y4YJQBA@mail.gmail.com>
Date: Thu, 2 Apr 2026 14:38:46 +0800
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <0BF8BC4F-ED7D-445F-8BB3-2336F1E17CB4@gmail.com>
References: 
 <CAApHDvq0T=iJ0Sf5TNE9yyWwfOeVjmrBt0wSywDnGD9Y4YJQBA@mail.gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/0BF8BC4F-ED7D-445F-8BB3-2336F1E17CB4%40gmail.com>
Precedence: bulk


> On Apr 2, 2026, at 12:09, David Rowley <dgrowleyml@gmail.com> wrote:
>=20
> I've been working on bms_left_shift_members() to bitshift members
> either left or right in order to tidy up some existing code and
> improve a future Bitmapset use case I'm currently working on.
>=20
> When testing some ERROR code I added to ensure we don't get an
> excessively large left shift value and end up with members higher than
> INT32_MAX, I discovered that bms_next_member() can't handle that
> value, as "prevbit++" will wrap to INT32_MIN and then we'll try to
> access a negative array index, i.e. seg fault.
>=20
> I appreciate that such a large member is quite unlikely, but if this
> isn't fixed then I need to code my error checking code to disallow
> members >=3D INT32_MAX rather than > INT32_MAX. I did have a comment
> explaining why I was doing that, but fixing the bug saves the weird
> special case and the comment.
>=20
> Patched attached. I was thinking it might not be worthy of
> backpatching, but I'll entertain alternative views on that.
>=20
> David
> <bms_next_member_fix.patch>

Both the return value of bms_next_member() and the parameter =E2=80=9Cprev=
bit" are defined as int, which seems to imply that a bitmap can hold at =
most INT32_MAX elements. So I wonder if a cleaner fix would be to just =
add a range guard, like this:

```
diff --git a/src/backend/nodes/bitmapset.c =
b/src/backend/nodes/bitmapset.c
index 786f343b3c9..7f79f81f278 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -1294,7 +1294,7 @@ bms_next_member(const Bitmapset *a, int prevbit)

        Assert(bms_is_valid_set(a));

-       if (a =3D=3D NULL)
+       if (a =3D=3D NULL || prevbit =3D=3D INT32_MAX)
                return -2;
        nwords =3D a->nwords;
        prevbit++;
```

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/