Skip to content

Commit

Permalink
Do not segfault on large histogram() parameters
Browse files Browse the repository at this point in the history
There is a bug in `width_bucket()` causing an overflow and subsequent
NaN value as a result of dividing with `+inf`. The NaN value is
interpreted as an integer and hence generates an index out of range for
the buckets.

This commit fixes this by generating an error rather than
segfaulting for bucket indexes that are out of range.
  • Loading branch information
mkindahl committed Mar 28, 2023
1 parent 22841ab commit 777c599
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/linux-32bit-build-and-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
DEBIAN_FRONTEND: noninteractive
IGNORES: "append-* debug_notice transparent_decompression-*
transparent_decompress_chunk-* plan_skip_scan-12 pg_dump"
SKIPS: chunk_adaptive
SKIPS: chunk_adaptive histogram_test
strategy:
fail-fast: false
matrix:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ accidentally triggering the load of a previous DB version.**
* #5446 Add checks for malloc failure in libpq calls
* #5470 Ensure superuser perms during copy/move chunk
* #5459 Fix issue creating dimensional constraints
* #5499 Do not segfault on large histogram() parameters

**Thanks**
* @nikolaps for reporting an issue with the COPY fetcher
* @S-imo-n for reporting the issue on Background Worker Scheduler crash
* @kovetskiy and @DZDomi for reporting peformance regression in Realtime Continuous Aggregates
* @geezhu for reporting issue on segfault in historgram()

## 2.10.1 (2023-03-07)

Expand Down
7 changes: 6 additions & 1 deletion src/histogram.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include "compat/compat.h"
#include "utils.h"
#include "debug_assert.h"

/* aggregate histogram:
* histogram(state, val, min, max, nbuckets) returns the histogram array with nbuckets
Expand Down Expand Up @@ -91,7 +92,11 @@ ts_hist_sfunc(PG_FUNCTION_ARGS)
Int32GetDatum(nbuckets)));

/* Increment the proper histogram bucket */
Assert(bucket < state->nbuckets);
if (bucket < 0 || bucket >= state->nbuckets)
ereport(ERROR,
(errcode(ERRCODE_INTERNAL_ERROR),
errmsg("index %d from \"width_bucket\" out of range", bucket),
errhint("You probably have a floating point overflow.")));
if (DatumGetInt32(state->buckets[bucket]) >= PG_INT32_MAX - 1)
elog(ERROR, "overflow in histogram");

Expand Down
28 changes: 28 additions & 0 deletions test/expected/histogram_test.out
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,31 @@ SELECT qualify, histogram(score, 0, 10, 5) FROM hitest2 GROUP BY qualify ORDER B
select histogram(i,10,90,case when i=1 then 1 else 1000000 end) FROM generate_series(1,100) i;
ERROR: number of buckets must not change between calls
\set ON_ERROR_STOP 1
CREATE TABLE weather (
time TIMESTAMPTZ NOT NULL,
city TEXT,
temperature FLOAT,
PRIMARY KEY(time, city)
);
-- There is a bug in width_bucket() causing a NaN as a result, so we
-- check that it is not causing a crash in histogram().
SELECT * FROM create_hypertable('weather', 'time', 'city', 3);
hypertable_id | schema_name | table_name | created
---------------+-------------+------------+---------
1 | public | weather | t
(1 row)

INSERT INTO weather VALUES
('2023-02-10 09:16:51.133584+00','city1',10.4),
('2023-02-10 11:16:51.611618+00','city1',10.3),
('2023-02-10 06:58:59.999999+00','city1',10.3),
('2023-02-10 01:58:59.999999+00','city1',10.3),
('2023-02-09 01:58:59.999999+00','city1',10.3),
('2023-02-10 08:58:59.999999+00','city1',10.3),
('2023-03-23 06:12:02.73765+00 ','city1', 9.7),
('2023-03-23 06:12:06.990998+00','city1',11.7);
-- This will currently generate an error.
\set ON_ERROR_STOP 0
SELECT histogram(temperature, -1.79769e+308, 1.79769e+308,10) FROM weather GROUP BY city;
ERROR: index -2147483648 from "width_bucket" out of range
\set ON_ERROR_STOP 1
25 changes: 25 additions & 0 deletions test/sql/histogram_test.sql
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,28 @@ SELECT qualify, histogram(score, 0, 10, 5) FROM hitest2 GROUP BY qualify ORDER B
\set ON_ERROR_STOP 0
select histogram(i,10,90,case when i=1 then 1 else 1000000 end) FROM generate_series(1,100) i;
\set ON_ERROR_STOP 1

CREATE TABLE weather (
time TIMESTAMPTZ NOT NULL,
city TEXT,
temperature FLOAT,
PRIMARY KEY(time, city)
);

-- There is a bug in width_bucket() causing a NaN as a result, so we
-- check that it is not causing a crash in histogram().
SELECT * FROM create_hypertable('weather', 'time', 'city', 3);
INSERT INTO weather VALUES
('2023-02-10 09:16:51.133584+00','city1',10.4),
('2023-02-10 11:16:51.611618+00','city1',10.3),
('2023-02-10 06:58:59.999999+00','city1',10.3),
('2023-02-10 01:58:59.999999+00','city1',10.3),
('2023-02-09 01:58:59.999999+00','city1',10.3),
('2023-02-10 08:58:59.999999+00','city1',10.3),
('2023-03-23 06:12:02.73765+00 ','city1', 9.7),
('2023-03-23 06:12:06.990998+00','city1',11.7);

-- This will currently generate an error.
\set ON_ERROR_STOP 0
SELECT histogram(temperature, -1.79769e+308, 1.79769e+308,10) FROM weather GROUP BY city;
\set ON_ERROR_STOP 1

0 comments on commit 777c599

Please sign in to comment.