Repository: incubator-impala Updated Branches: refs/heads/master 2544f2394 -> c27ad3526
IMPALA-5088: Fix heap buffer overflow In a recent patch (IMPALA-4787), we introduced a change where we are reading past the end of a buffer during a copy. Even though we would never read the data that was copied from past the end of the buffer, this could cause a crash if the allocation is sitting on the edge of an unmapped area of memory. This also caused the ASAN build to fail. The issue is fixed by never accessing memory that is past the end of the buffer. Change-Id: Ia68d69b35d53934e6f1aae2e1696503d17660ca2 Reviewed-on: http://gerrit.cloudera.org:8080/6441 Reviewed-by: Taras Bobrovytsky <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/c27ad352 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c27ad352 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c27ad352 Branch: refs/heads/master Commit: c27ad3526675ff0ab80c7bb5d7c8364437448b53 Parents: 2544f23 Author: Taras Bobrovytsky <[email protected]> Authored: Mon Mar 20 15:05:17 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Mar 21 09:13:08 2017 +0000 ---------------------------------------------------------------------- be/src/exprs/aggregate-functions-ir.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c27ad352/be/src/exprs/aggregate-functions-ir.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/aggregate-functions-ir.cc b/be/src/exprs/aggregate-functions-ir.cc index fc4715a..72a78b9 100644 --- a/be/src/exprs/aggregate-functions-ir.cc +++ b/be/src/exprs/aggregate-functions-ir.cc @@ -1066,10 +1066,13 @@ class ReservoirSampleState { size_t buffer_len = sizeof(ReservoirSampleState<T>) + sizeof(ReservoirSample<T>) * num_samples_; - StringVal dst = StringVal::CopyFrom( - ctx, reinterpret_cast<uint8_t*>(this), buffer_len); - memcpy(dst.ptr + sizeof(ReservoirSampleState<T>), - reinterpret_cast<uint8_t*>(samples_), sizeof(ReservoirSample<T>) * num_samples_); + StringVal dst(ctx, buffer_len); + if (LIKELY(!dst.is_null)) { + memcpy(dst.ptr, reinterpret_cast<uint8_t*>(this), sizeof(ReservoirSampleState<T>)); + memcpy(dst.ptr + sizeof(ReservoirSampleState<T>), + reinterpret_cast<uint8_t*>(samples_), + sizeof(ReservoirSample<T>) * num_samples_); + } ctx->Free(reinterpret_cast<uint8_t*>(samples_)); ctx->Free(reinterpret_cast<uint8_t*>(this)); return dst;
