Re: BUG #17280: global-buffer-overflow on select from pg_stat_slru

At Wed, 10 Nov 2021 13:42:25 +0000, PG Bug reporting form <noreply(at)postgresql(dot)org> wrote in
> The following bug has been logged on the website:
>
> Bug reference: 17280
> Logged by: Alexander Kozhemyakin
> Email address: a(dot)kozhemyakin(at)postgrespro(dot)ru
> PostgreSQL version: 14.0
> Operating system: Ubuntu 21.04
> Description:
>
> The following simple query:
> select * from pg_catalog.pg_stat_slru
> leads to the sanitizer-detected error:
> ==23911==ERROR: AddressSanitizer: global-buffer-overflow on address
> 0x5582bec7c5e0 at pc 0x5582bbd2c01c bp 0x7fff0b73a470 sp 0x7fff0b73a460
> READ of size 64 at 0x5582bec7c5e0 thread T0
> #0 0x5582bbd2c01b in pg_stat_get_slru
> /home/postgres/postgres/src/backend/utils/adt/pgstatfuncs.c:1914

slruStats has 8 elements (SLRU_NUM_ELEMENTS).

In the loop in pg_stat_get_slru digested as below,

> stats = pgstat_fetch_slru(); - returns slruStats
>
> for (i = 0;; i++)
> {
!> PgStat_SLRUStats stat = stats[i];
> name = pgstat_slru_name(i);
>
> if (!name)
> break;

The line prefixed by '!' runs with i = 8, which is actually overrun.

I see three ways to fix it. One is to move the assignment to stat to
after the break. Another is to limit the for loop using
SLRU_NUM_ELEMENTS. The last one is limit the for loop using
pgstat_slru_name().

The loop is designed not to directly rely on SRLU_NUM_ELEMENTS so if
we honor that design, we would take the first or the third way. The
first way is smallest but I prefer the third way as it is
straightforward as such kind of loops. The attached is that for the
master.

The code was introduced at 13 and the attached applies to the versions
back to 13.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center