Re: Small and unlikely overflow hazard in bms_next_member()

Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> 于2026年4月2日周四 14:39写道：

>
>
> > On Apr 2, 2026, at 12:09, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> >
> > 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.
> >
> > 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.
> >
> > 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 >= 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.
> >
> > Patched attached. I was thinking it might not be worthy of
> > backpatching, but I'll entertain alternative views on that.
> >
> > David
> > <bms_next_member_fix.patch>
>
> Both the return value of bms_next_member() and the parameter “prevbit" 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 == NULL)
> + if (a == NULL || prevbit == INT32_MAX)
> return -2;
> nwords = a->nwords;
> prevbit++;
> ```
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
>
>
>
>
>
Hi,

The both solutions look good to me. I am slightly keen on Chao's version
that looks simpler to me.

Thanks!
wang jie