Re: Fix possible bogus array out of bonds (src/backend/access/brin/brin_minmax_multi.c)

On Mon, Aug 29, 2022 at 10:06:55AM +0900, Kyotaro Horiguchi wrote:
> At Fri, 26 Aug 2022 10:28:50 -0300, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote in
> > At function has_matching_range, if variable ranges->nranges == 0,
> > we exit quickly with a result equal to false.
> >
> > This means that nranges can be zero.
> > It occurs then that it is possible then to occur an array out of bonds, in
> > the initialization of the variable maxvalue.
> > So if nranges is equal to zero, there is no need to initialize minvalue and
> > maxvalue.
> >
> > The patch tries to fix it, avoiding possible errors by using maxvalue.
>
> However it seems that nranges will never be zero, still the fix looks
> good to me since it is generally allowed to be zero. I don't find a
> similar mistake related to Range.nranges.

Actually, the nranges==0 branch is hit during regression tests:
https://coverage.postgresql.org/src/backend/access/brin/brin_minmax_multi.c.gcov.html

I'm not sure, but I *suspect* that compilers usually check
ranges->nranges==0
before reading ranges->values[2 * ranges->nranges - 1];

Especially since it's a static function.

Even if they didn't (say, under -O0), values[-1] would probably point to
a palloc header, which would be enough to "not crash" before returning
one line later.

But +1 to fix this and other issues even if they would never crash.

--
Justin