Re: BRIN integer overflow

2024-02-21 Thread Daniel Gustafsson
> On 21 Feb 2024, at 06:40, Oleg Tselebrovskiy  
> wrote:

> Function bringetbitmap that is used in BRIN's IndexAmRoutine should return an
> int64 value, but the actual return value is int, since totalpages is int and
> totalpages * 10 is also int. This could lead to integer overflow

(totalpages * 10) overflowing an int seems like a quite theoretical risk which
would be hard to hit in practice.

> I suggest to change totalpages to be int64 to avoid potential overflow.
> Also in all other "amgetbitmap functions" (such as hashgetbitmap, 
> gistgetbitmap,
> gingetbitmap, blgetbitmap) the return value is of correct int64 type

That being said, changing it like this seems reasonable since the API is
defined as int64, and it will keep static analyzers quiet.

--
Daniel Gustafsson





BRIN integer overflow

2024-02-20 Thread Oleg Tselebrovskiy

Greetings, everyone!

While analyzing output of Svace static analyzer [1] I've found a bug

Function bringetbitmap that is used in BRIN's IndexAmRoutine should 
return an
int64 value, but the actual return value is int, since totalpages is int 
and

totalpages * 10 is also int. This could lead to integer overflow

I suggest to change totalpages to be int64 to avoid potential overflow.
Also in all other "amgetbitmap functions" (such as hashgetbitmap, 
gistgetbitmap,

gingetbitmap, blgetbitmap) the return value is of correct int64 type

The proposed patch is attached

[1] - https://svace.pages.ispras.ru/svace-website/en/

Oleg Tselebrovskiy, Postgres Prodiff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 1087a9011ea..f72667e484e 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -559,7 +559,7 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 	BrinOpaque *opaque;
 	BlockNumber nblocks;
 	BlockNumber heapBlk;
-	int			totalpages = 0;
+	int64		totalpages = 0;
 	FmgrInfo   *consistentFn;
 	MemoryContext oldcxt;
 	MemoryContext perRangeCxt;