IMPALA-3302: Fix ASAN use-after-free failure in Kudu scanner When fetching VARCHAR data from Kudu, Impala was using a pointer that was owned and freed by the Kudu client. There was already a mechanism to copy string data but it was only applied to STRING columns. The fix is to apply the same mechanism to VARCHAR columns. That's done by adding the VARCHAR column to the list of string type columns/slots that need to be copied.
Change-Id: Ib513aae531086355e77ac9c35ed38728efaa0be9 Reviewed-on: http://gerrit.cloudera.org:8080/2749 Reviewed-by: Dan Hecht <[email protected]> Tested-by: Internal 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/62bb8647 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/62bb8647 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/62bb8647 Branch: refs/heads/master Commit: 62bb864704d5838726095bf90aeec1ba2c0ef881 Parents: 36c294b Author: casey <[email protected]> Authored: Sun Apr 10 23:20:55 2016 -0700 Committer: Tim Armstrong <[email protected]> Committed: Tue Apr 12 14:03:44 2016 -0700 ---------------------------------------------------------------------- be/src/exec/kudu-scanner.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/62bb8647/be/src/exec/kudu-scanner.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/kudu-scanner.cc b/be/src/exec/kudu-scanner.cc index a89a7db..f0d0f57 100644 --- a/be/src/exec/kudu-scanner.cc +++ b/be/src/exec/kudu-scanner.cc @@ -88,7 +88,7 @@ Status KuduScanner::Open() { // Store columns that need relocation when materialized into the // destination row batch. for (int i = 0; i < scan_node_->tuple_desc_->slots().size(); ++i) { - if (scan_node_->tuple_desc_->slots()[i]->type().type == TYPE_STRING) { + if (scan_node_->tuple_desc_->slots()[i]->type().IsStringType()) { string_slots_.push_back(scan_node_->tuple_desc_->slots()[i]); } } @@ -202,10 +202,12 @@ Status KuduScanner::DecodeRowsIntoRowBatch(RowBatch* row_batch, Tuple** tuple_mem, bool* batch_done) { // Short-circuit the count(*) case. - if (scan_node_->tuple_desc_->slots().empty()) return HandleEmptyProjection(row_batch, batch_done); + if (scan_node_->tuple_desc_->slots().empty()) { + return HandleEmptyProjection(row_batch, batch_done); + } - // TODO consider consolidating the tuple creation/initialization here with the version that - // happens inside the loop. + // TODO consider consolidating the tuple creation/initialization here with the version + // that happens inside the loop. int idx = row_batch->AddRow(); TupleRow* row = row_batch->GetRow(idx); (*tuple_mem)->Init(scan_node_->tuple_desc()->num_null_bytes()); @@ -265,7 +267,7 @@ Status KuduScanner::RelocateValuesFromKudu(Tuple* tuple, MemPool* mem_pool) { // Extract the string value. void* slot_ptr = tuple->GetSlot(slot->tuple_offset()); - DCHECK(slot->type().IsStringType()); + DCHECK(slot->type().IsVarLenStringType()); // The string value of the slot has a pointer to memory from the Kudu row. StringValue* val = reinterpret_cast<StringValue*>(slot_ptr);
