Re: BUG #14722: Segfault in tuplesort_heap_siftup, 32 bit overflow
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Sergey Koposov <skoposov@cmu.edu>
Cc: "pg@bowt.ie" <pg@bowt.ie>,
"pgsql-bugs@postgresql.org" <pgsql-bugs@postgresql.org>
Date: 2017-07-05T22:03:56Z
Lists: pgsql-bugs
Sergey Koposov <skoposov@cmu.edu> writes: > On Thu, 2017-06-29 at 10:00 -0700, Peter Geoghegan wrote: >> This is an oversight in commit 263865a. The fix is to use a variable >> that won't overflow in tuplesort_heap_siftup() -- this is probably a >> one-liner, because when the variable overflows today, the correct >> behavior would be for control to break out of the loop that declares >> the overflowing variable "j", and, I don't see any similar problem in >> other heap maintenance routines. It's a very isolated problem. >> >> I could write a patch. > Just to avoid being forgotten, I attach a trivial patch against 9.5 > branch as well as have created a commitfest submission > https://commitfest.postgresql.org/14/1189/ I don't like s/int/int64/g as a fix for this. That loop is probably a hot spot, and this fix is going to be expensive on any machine where int64 isn't the native word width. How about something like this instead: - int j = 2 * i + 1; + int j; + if (unlikely(i > INT_MAX / 2)) + break; /* if j would overflow, we're done */ + j = 2 * i + 1; if (j >= n) break; regards, tom lane
Commits
-
Avoid integer overflow while sifting-up a heap in tuplesort.c.
- e439bbe9996f 9.4.13 landed
- e7213fe2bda8 9.5.8 landed
- 512f67c8d02c 10.0 landed
- 09c598898166 9.6.4 landed
-
Implement binary heap replace-top operation in a smarter way.
- 24598337c8d2 10.0 cited
-
Permit super-MaxAllocSize allocations with MemoryContextAllocHuge().
- 263865a48973 9.4.0 cited