From a916cb9d5a89804998dd4e7fd7bbb27cb5a7abc8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 21 May 2022 13:13:41 -0400 Subject: Avoid overflow hazard when clamping group counts to "long int". Several places in the planner tried to clamp a double value to fit in a "long" by doing (long) Min(x, (double) LONG_MAX); This is subtly incorrect, because it casts LONG_MAX to double and potentially back again. If long is 64 bits then the double value is inexact, and the platform might round it up to LONG_MAX+1 resulting in an overflow and an undesirably negative output. While it's not hard to rewrite the expression into a safe form, let's put it into a common function to reduce the risk of someone doing it wrong in future. In principle this is a bug fix, but since the problem could only manifest with group count estimates exceeding 2^63, it seems unlikely that anyone has actually hit this or will do so anytime soon. We're fixing it mainly to satisfy fuzzer-type tools. That being the case, a HEAD-only fix seems sufficient. Andrey Lepikhov Discussion: https://postgr.es/m/ebbc2efb-7ef9-bf2f-1ada-d6ec48f70e58@postgrespro.ru --- src/backend/optimizer/plan/createplan.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src/backend/optimizer/plan/createplan.c') diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index f4cc56039c2..76606faa3e4 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -16,7 +16,6 @@ */ #include "postgres.h" -#include #include #include "access/sysattr.h" @@ -2724,7 +2723,7 @@ create_setop_plan(PlannerInfo *root, SetOpPath *best_path, int flags) flags | CP_LABEL_TLIST); /* Convert numGroups to long int --- but 'ware overflow! */ - numGroups = (long) Min(best_path->numGroups, (double) LONG_MAX); + numGroups = clamp_cardinality_to_long(best_path->numGroups); plan = make_setop(best_path->cmd, best_path->strategy, @@ -2761,7 +2760,7 @@ create_recursiveunion_plan(PlannerInfo *root, RecursiveUnionPath *best_path) tlist = build_path_tlist(root, &best_path->path); /* Convert numGroups to long int --- but 'ware overflow! */ - numGroups = (long) Min(best_path->numGroups, (double) LONG_MAX); + numGroups = clamp_cardinality_to_long(best_path->numGroups); plan = make_recursive_union(tlist, leftplan, @@ -6554,7 +6553,7 @@ make_agg(List *tlist, List *qual, long numGroups; /* Reduce to long, but 'ware overflow! */ - numGroups = (long) Min(dNumGroups, (double) LONG_MAX); + numGroups = clamp_cardinality_to_long(dNumGroups); node->aggstrategy = aggstrategy; node->aggsplit = aggsplit; -- cgit v1.2.3