From 18afe429a42bae11f5976c0c2736e7b25264ae49 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 23 May 2011 12:53:00 -0400 Subject: [PATCH] Install defenses against overflow in BuildTupleHashTable(). The planner can sometimes compute very large values for numGroups, and in cases where we have no alternative to building a hashtable, such a value will get fed directly to BuildTupleHashTable as its nbuckets parameter. There were two ways in which that could go bad. First, BuildTupleHashTable declared the parameter as "int" but most callers were passing "long"s, so on 64-bit machines undetected overflow could occur leading to a bogus negative value. The obvious fix for that is to change the parameter to "long", which is what I've done in HEAD. In the back branches that seems a bit risky, though, since third-party code might be calling this function. So for them, just put in a kluge to treat negative inputs as INT_MAX. Second, hash_create can go nuts with extremely large requested table sizes (notably, my_log2 becomes an infinite loop for inputs larger than LONG_MAX/2). What seems most appropriate to avoid that is to bound the initial table size request to work_mem. This fixes bug #6035 reported by Daniel Schreiber. Although the reported case only occurs back to 8.4 since it involves WITH RECURSIVE, I think it's a good idea to install the defenses in all supported branches. --- src/backend/executor/execGrouping.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/execGrouping.c b/src/backend/executor/execGrouping.c index 85269bfbac..a21af0fe1d 100644 --- a/src/backend/executor/execGrouping.c +++ b/src/backend/executor/execGrouping.c @@ -15,6 +15,7 @@ #include "postgres.h" #include "executor/executor.h" +#include "miscadmin.h" #include "parser/parse_oper.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -278,9 +279,20 @@ BuildTupleHashTable(int numCols, AttrNumber *keyColIdx, TupleHashTable hashtable; HASHCTL hash_ctl; - Assert(nbuckets > 0); + /* + * Many callers pass "long" values for nbuckets, which means that we can + * receive a bogus value on 64-bit machines. It seems unwise to change + * this function's signature in released branches, so instead assume that + * a negative input means long->int overflow occurred. + */ + if (nbuckets <= 0) + nbuckets = INT_MAX; + Assert(entrysize >= sizeof(TupleHashEntryData)); + /* Limit initial table size request to not more than work_mem */ + nbuckets = Min(nbuckets, (long) ((work_mem * 1024L) / entrysize)); + hashtable = (TupleHashTable) MemoryContextAlloc(tablecxt, sizeof(TupleHashTableData));