Re: [PATCH] bms_prev_member() can read beyond the end of the array of allocated words
Greg Burd <greg@burd.me>
From: Greg Burd <greg@burd.me>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Rowley <dgrowleyml@gmail.com>, PostgreSQL Hackers
<pgsql-hackers@lists.postgresql.org>
Date: 2025-08-14T15:37:28Z
Lists: pgsql-hackers
Attachments
- v3-0001-Prevent-bms_prev_member-from-reading-beyond-the-e.patch (application/octet-stream) patch v3-0001
On Aug 14 2025, at 11:14 am, Tom Lane <tgl@sss.pgh.pa.us> wrote: > David Rowley <dgrowleyml@gmail.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 Thanks Tom, David, Seems I also forgot about the case where the Bitmapset passed is NULL. The new assert needs to handle that as well. -greg
Commits
-
Add Asserts to validate prevbit values in bms_prev_member
- b4632883d44e 19 (unreleased) landed