On Wed, Sep 04, 2024 at 03:14:34PM -0300, Ranier Vilela wrote: > The commit 7949d95 <http://7949d9594582ab49dee221e1db1aa5401ace49d4>, left > out an oversight. > > The report is: > CID 1559468: (#1 of 1): Overflowed array index read (INTEGER_OVERFLOW) > > I think that Coverity is right. > In the function *pgstat_read_statsfile* It is necessary to first check > whether it is the most restrictive case. > > Otherwise, if PgStat_Kind is greater than 11, a negative index may occur.
You are missing the fact that there is a call to pgstat_is_kind_valid() a couple of lines above, meaning that we are sure that the kind ID we are dealing with is within the range [1,11] for built-in kinds or [128,256] for the custom kinds, so any ID not within the first range would just be within the second range. Speaking of which, I am spotting two possible pointer dereferences when reading the stats file if we are loading custom stats that do not exist anymore compared to the moment when they were written, for two cases: - Fixed-numbered stats entries. - Named entries, like replication slot stats, but for the custom case. It would mean that we'd crash at startup when reading stats depending on how shared_preload_libraries has changed, which is not the original intention. The patch includes details to inform what was found wrong with two new WARNING messages. Will fix in a bit, attaching it for now. Kind of interesting that your tool did not spot that, and missed the two I have noticed considering that we're dealing with the same code paths. The community coverity did not complain on any of them, AFAIK. -- Michael
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index b2ca3f39b7..612158f2b9 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -1781,6 +1781,12 @@ pgstat_read_statsfile(XLogRecPtr redo) } info = pgstat_get_kind_info(kind); + if (!info) + { + elog(WARNING, "could not find information of kind %u for entry of type %c", + kind, t); + goto error; + } if (!info->fixed_amount) { @@ -1861,6 +1867,12 @@ pgstat_read_statsfile(XLogRecPtr redo) } kind_info = pgstat_get_kind_info(kind); + if (!kind_info) + { + elog(WARNING, "could not find information of kind %u for entry of type %c", + kind, t); + goto error; + } if (!kind_info->from_serialized_name) {
signature.asc
Description: PGP signature