At Thu, 11 Nov 2021 11:52:27 +0900, Michael Paquier <michael@paquier.xyz> wrote in
> On Thu, Nov 11, 2021 at 10:39:23AM +0900, Kyotaro Horiguchi wrote:
> > 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.
>
> Or it would be easier for the reader to assign stat after checking for
> the result of pgstat_slru_name(), no? I am not much a fan of this
> code style that uses a counter, FWIW, but at the same time
> SLRU_NUM_ELEMENTS is local to pgstat.c, so..
I'm not sure which is easier to read, but it might be a bit hard since
the conditino term in not mention counter itself. I don't object to
that way. And, yes SLRU_NUM_ELEMENTS cannot be used here:p
The attached is the first way in the choices.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 6ddc188713f6314e25a78ec4c4f095376e6263c6 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 11 Nov 2021 12:09:45 +0900
Subject: [PATCH v2] Fix memory overrun of pg_stat_get_slru
The function accesses one element after the end of an array, by
accessing the array using a loop variable before exiting a loop.
Avoid that access by accessing the elements after the check against
the exit condition.
Backpatch to 13, where slru stats was introduced.
---
src/backend/utils/adt/pgstatfuncs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index ff5aedc99c..e64857e540 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1911,7 +1911,7 @@ pg_stat_get_slru(PG_FUNCTION_ARGS)
/* for each row */
Datum values[PG_STAT_GET_SLRU_COLS];
bool nulls[PG_STAT_GET_SLRU_COLS];
- PgStat_SLRUStats stat = stats[i];
+ PgStat_SLRUStats stat;
const char *name;
name = pgstat_slru_name(i);
@@ -1919,6 +1919,7 @@ pg_stat_get_slru(PG_FUNCTION_ARGS)
if (!name)
break;
+ stat = stats[i];
MemSet(values, 0, sizeof(values));
MemSet(nulls, 0, sizeof(nulls));
--
2.27.0