From 7a09eb8ea4ed7a5759a8d5f4a4be277c6ca98b72 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Thu, 29 Sep 2016 14:00:28 +0100 Subject: [PATCH] Fix use-after-free around DISTINCT transition function calls. process_ordered_aggregate_multi() failed to respect an undocumented requirement to not reuse tuplesort slot contents across calls to tuplesort_gettupleslot(). This could lead to wrong answers to queries, or a backend crash, since memory could be recycled or freed; tuple slots used and reused as the transition function is advanced could still hold dangling pointers. To fix, have process_ordered_aggregate_multi() copy the contents of its current table slot to effectively persist the contained tuple across calls to tuplesort_gettupleslot(). Also, update tuplesort_gettupleslot() contract to make this requirement clear. Per bug #14344 from Regina Obe. Report: <20160929035538.20224.39628@wrigleys.postgresql.org> Backpatch-through: 9.6 --- src/backend/executor/nodeAgg.c | 6 ++---- src/backend/utils/sort/tuplesort.c | 4 ++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index ce2fc28..8f2c8f4 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1259,11 +1259,9 @@ process_ordered_aggregate_multi(AggState *aggstate, if (numDistinctCols > 0) { - /* swap the slot pointers to retain the current tuple */ - TupleTableSlot *tmpslot = slot2; + /* replace slot2 contents to retain the current tuple */ - slot2 = slot1; - slot1 = tmpslot; + ExecCopySlot(slot2, slot1); /* avoid execTuplesMatch() calls by reusing abbreviated keys */ oldAbbrevVal = newAbbrevVal; haveOldValue = true; diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index 16ceb30..99eec40 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -2054,6 +2054,10 @@ tuplesort_gettuple_common(Tuplesortstate *state, bool forward, * determination of "non-equal tuple" based on simple binary inequality. A * NULL value in leading attribute will set abbreviated value to zeroed * representation, which caller may rely on in abbreviated inequality check. + * + * Caller should not rely on contents of slot remaining allocated following + * next call here. A call to ExecCopySlot() may be required to make slot + * contents persist across calls here. */ bool tuplesort_gettupleslot(Tuplesortstate *state, bool forward, -- 2.7.4