Re: pg_stat_get_activity(): integer overflow due to (int) * (int) for MemoryContextAllocHuge()

On Thu, Sep 28, 2023 at 11:01:14AM +0200, Jakub Wartak wrote:
> v3 attached. I had a problem coming out with a better error message,
> so suggestions are welcome. The cast still needs to be present as per
> above suggestion as 3GB is still valid buf size and still was causing
> integer overflow. We just throw an error on >= 4GB with v3.

+/* Safety net to prevent requesting huge memory by each query to pg_stat_activity */
+#define PGSTAT_MAX_ACTIVITY_BUF_SIZE 4 * 1024 * 1024 * 1024L

- size = add_size(size,
- mul_size(pgstat_track_activity_query_size, NumBackendStatSlots));
+ pgstat_track_size = mul_size(pgstat_track_activity_query_size,
+ NumBackendStatSlots);
+ if(pgstat_track_size >= PGSTAT_MAX_ACTIVITY_BUF_SIZE)
+ elog(FATAL, "too big Backend Activity Buffer allocation of %zu bytes", pgstat_track_size);
+ size = add_size(size, pgstat_track_size);

That should be enough to put in a hardcoded 4GB safety limit, while
mul_size() detects it at a higher range. Note, however, that elog()
is only used for internal errors that users should never face, but
this one can come from a misconfiguration. This would be better as an
ereport(), with ERRCODE_CONFIG_FILE_ERROR as errcode, I guess.

"Backend Activity Buffer" is the name of the internal struct. Sure,
it shows up on the system views for shmem, but I wouldn't use this
term in a user-facing error message. Perhaps something like "size
requested for backend status is out of range" would be cleaner. Other
ideas are welcome.
--
Michael