[PATCH] Fix possible overflow on tuplesort.c
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t42244psql -h localhost -U postgresBuilt from patchset v1 (message #1), September 20, 2026 at 12:18 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t42244_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t42244_1 && git checkout t42244_1Patchset v1 (message #1) is on t42244_1
Hi,
When multiplying variables, the overflow will take place anyway, and only
then will the meaningless product be explicitly promoted to type int64.
It is one of the operands that should have been cast instead to avoid the
overflow.
regards,
Ranier Vilela
On 2020-Apr-16, Ranier Vilela wrote:
When multiplying variables, the overflow will take place anyway, and only
then will the meaningless product be explicitly promoted to type int64.
It is one of the operands that should have been cast instead to avoid the
overflow.- if (state->availMem < (int64) ((newmemtupsize - memtupsize) * sizeof(SortTuple))) + if (state->availMem < ((int64) (newmemtupsize - memtupsize) * sizeof(SortTuple)))
Doesn't sizeof() return a 64-bit wide value already?
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
When multiplying variables, the overflow will take place anyway, and only
then will the meaningless product be explicitly promoted to type int64.
It is one of the operands that should have been cast instead to avoid the
overflow.- if (state->availMem < (int64) ((newmemtupsize - memtupsize) * sizeof(SortTuple))) + if (state->availMem < ((int64) (newmemtupsize - memtupsize) * sizeof(SortTuple)))
Doesn't sizeof() return a 64-bit wide value already?
Not on 32-bit machines. However, on a 32-bit machine the clamp just
above here would prevent overflow anyway. In general, said clamp
ensures that the value computed here is less than MaxAllocHugeSize,
so computing it in size_t width is enough. So in fact an overflow is
impossible here, but it requires looking at more than this one line of
code to see it. I would expect a static analyzer to understand it though.
I think the actual point of this cast is to ensure that the comparison to
availMem is done in signed not unsigned arithmetic --- which is critical
because availMem might be negative. The proposed change would indeed
break that, since multiplying a signed value by size_t is presumably going
to produce an unsigned value. We could use two casts, but I don't see the
point.
regards, tom lane
Em qui., 23 de abr. de 2020 às 16:43, Alvaro Herrera <
alvherre@2ndquadrant.com> escreveu:
On 2020-Apr-16, Ranier Vilela wrote:
When multiplying variables, the overflow will take place anyway, and only
then will the meaningless product be explicitly promoted to type int64.
It is one of the operands that should have been cast instead to avoid the
overflow.- if (state->availMem < (int64) ((newmemtupsize - memtupsize) *
sizeof(SortTuple)))
+ if (state->availMem < ((int64) (newmemtupsize - memtupsize) *
sizeof(SortTuple)))
Doesn't sizeof() return a 64-bit wide value already?
Sizeof return size_t.
Both versions are constant expressions of type std::size_t
<https://en.cppreference.com/w/cpp/types/size_t>.
regards,
Ranier Vilela