Re: [PATCH] bms_prev_member() can read beyond the end of the array of allocated words

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Greg Burd <greg(at)burd(dot)me>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] bms_prev_member() can read beyond the end of the array of allocated words
Date: 2025-08-14 15:14:49
Message-ID: 138396.1755184489@sss.pgh.pa.us
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> It is valid to pass prevbit as a->nwords * BITS_PER_BITMAPWORD as the
> code does "prevbit--;". Maybe it would be less confusing if it were
> written as:
> * "prevbit" must be less than or equal to "a->nwords * BITS_PER_BITMAPWORD".
> The Assert should be using <= rather than <.

Actually, I don't agree with that. It's true that it wouldn't fail,
but a caller doing that is exhibiting undue intimacy with the innards
of Bitmapsets. The expected usage is that the argument is initially
-1 and after that the result of the previous call (which'll
necessarily be less than a->nwords * BITS_PER_BITMAPWORD). We don't
have any state with which we can verify the chain of calls, but it
seems totally reasonable to me to disallow an outside caller
providing an argument >= a->nwords * BITS_PER_BITMAPWORD.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2025-08-14 15:20:13 Re: Annoying warning in SerializeClientConnectionInfo
Previous Message KAZAR Ayoub 2025-08-14 14:59:55 Re: Speed up COPY FROM text/CSV parsing using SIMD