[2/3] incubator-impala git commit: IMPALA-5261: Heap use-after-free in HdfsSequenceTableWriter

[kw...@apache.org]

IMPALA-5261: Heap use-after-free in HdfsSequenceTableWriter

HdfsSequenceTableWriter::ConsumeRow() function dereferenced a pointer
that pointed to a previously deallocated memory (which belonged to an
out of scope string object). This caused the ASAN build to fail.

The fix was verified by running TestTableWriters.test_seq_writer and
TestTableWriters.test_seq_writer_hive_compatibility end-to-end tests
against the ASAN build. These tests consistently crashed impalad
before the fix.

Change-Id: Id339247f892710529d8ad56dd1e98eadbf32900b
Reviewed-on: http://gerrit.cloudera.org:8080/6762
Reviewed-by: Michael Ho <kw...@cloudera.com>
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/c1be7745
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c1be7745
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c1be7745

Branch: refs/heads/master
Commit: c1be77458be5f57b23ae70468873bfb70be5c5e8
Parents: 741421d
Author: Attila Jeges <at...@cloudera.com>
Authored: Fri Apr 28 22:54:19 2017 +0200
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat Apr 29 02:54:42 2017 +0000

----------------------------------------------------------------------
 be/src/exec/hdfs-sequence-table-writer.cc | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c1be7745/be/src/exec/hdfs-sequence-table-writer.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-sequence-table-writer.cc b/be/src/exec/hdfs-sequence-table-writer.cc
index 9af7e0f..a05aa5d 100644
--- a/be/src/exec/hdfs-sequence-table-writer.cc
+++ b/be/src/exec/hdfs-sequence-table-writer.cc
@@ -284,13 +284,13 @@ inline Status HdfsSequenceTableWriter::ConsumeRow(TupleRow* row) {
 
   const uint8_t* value_bytes;
   int64_t value_length;
+  string text = row_buf_.String();
   if (compress_flag_) {
     // apply compression to row_buf_
     // the length of the buffer must be prefixed to the buffer prior to compression
     //
     // TODO this incurs copy overhead to place the length in front of the
     // buffer prior to compression. We may want to rewrite to avoid copying.
-    string text = row_buf_.String();
     row_buf_.Clear();
     // encoding as "Text" writes the length before the text
     row_buf_.WriteText(text.size(), reinterpret_cast<const uint8_t*>(&text.data()[0]));
@@ -303,8 +303,9 @@ inline Status HdfsSequenceTableWriter::ConsumeRow(TupleRow* row) {
     }
     value_bytes = tmp;
   } else {
-    value_length = row_buf_.Size();
-    value_bytes = reinterpret_cast<const uint8_t*>(row_buf_.String().data());
+    value_length = text.size();
+    DCHECK_EQ(value_length, row_buf_.Size());
+    value_bytes = reinterpret_cast<const uint8_t*>(text.data());
   }
 
   int rec_len = value_length;