From 082c0c36283b4d91e30051eb70b048f16570c873 Mon Sep 17 00:00:00 2001 From: Yingchun Lai <405403881@qq.com> Date: Wed, 26 Aug 2020 09:57:00 +0000 Subject: [PATCH 1/2] [Bug] Fix bug that memory copy may overflow in MemIndex::load_segment --- be/src/olap/olap_index.cpp | 21 ++++++++++++++++++++- be/src/olap/rowset/segment_group.cpp | 1 + 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/be/src/olap/olap_index.cpp b/be/src/olap/olap_index.cpp index 4b549030de1a1d..0b3930850e8c35 100644 --- a/be/src/olap/olap_index.cpp +++ b/be/src/olap/olap_index.cpp @@ -208,6 +208,11 @@ OLAPStatus MemIndex::load_segment(const char* file, size_t *current_num_rows_per memory_copy(mem_ptr, storage_ptr, null_byte); // 2. copy length and content + bool is_null = *reinterpret_cast(mem_ptr); + if (is_null) { + continue; + } + size_t storage_field_bytes = *reinterpret_cast(storage_ptr + null_byte); Slice* slice = reinterpret_cast(mem_ptr + 1); @@ -235,6 +240,11 @@ OLAPStatus MemIndex::load_segment(const char* file, size_t *current_num_rows_per memory_copy(mem_ptr, storage_ptr, null_byte); // 2. copy length and content + bool is_null = *reinterpret_cast(mem_ptr); + if (is_null) { + continue; + } + Slice* slice = reinterpret_cast(mem_ptr + 1); char* data = reinterpret_cast(_mem_pool->allocate(storage_field_bytes)); memory_copy(data, storage_ptr + null_byte, storage_field_bytes); @@ -248,7 +258,16 @@ OLAPStatus MemIndex::load_segment(const char* file, size_t *current_num_rows_per size_t storage_field_bytes = column.index_length(); mem_field_offset += storage_field_bytes + 1; for (size_t j = 0; j < num_entries; ++j) { - memory_copy(mem_ptr + 1 - null_byte, storage_ptr, storage_field_bytes + null_byte); + // 1. copy null_byte + memory_copy(mem_ptr, storage_ptr, null_byte); + + // 2. copy content + bool is_null = *reinterpret_cast(mem_ptr); + if (is_null) { + continue; + } + + memory_copy(mem_ptr + 1, storage_ptr + null_byte, storage_field_bytes); mem_ptr += mem_row_bytes; storage_ptr += storage_row_bytes; diff --git a/be/src/olap/rowset/segment_group.cpp b/be/src/olap/rowset/segment_group.cpp index 7829488f781119..eff48c0b5162ef 100644 --- a/be/src/olap/rowset/segment_group.cpp +++ b/be/src/olap/rowset/segment_group.cpp @@ -577,6 +577,7 @@ OLAPStatus SegmentGroup::add_segment() { return OLAP_ERR_MALLOC_ERROR; } + memset(_short_key_buf, 0, _short_key_length); if (_current_index_row.init(*_schema) != OLAP_SUCCESS) { OLAP_LOG_WARNING("init _current_index_row fail."); return OLAP_ERR_INIT_FAILED; From 3c7fe36d3203f54f9b62aa9bfeda7d1af7b2a9b7 Mon Sep 17 00:00:00 2001 From: Yingchun Lai <405403881@qq.com> Date: Thu, 27 Aug 2020 12:05:09 +0000 Subject: [PATCH 2/2] fix --- be/src/olap/olap_index.cpp | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/be/src/olap/olap_index.cpp b/be/src/olap/olap_index.cpp index 0b3930850e8c35..bd9ac44b91cd2f 100644 --- a/be/src/olap/olap_index.cpp +++ b/be/src/olap/olap_index.cpp @@ -209,18 +209,16 @@ OLAPStatus MemIndex::load_segment(const char* file, size_t *current_num_rows_per // 2. copy length and content bool is_null = *reinterpret_cast(mem_ptr); - if (is_null) { - continue; + if (!is_null) { + size_t storage_field_bytes = + *reinterpret_cast(storage_ptr + null_byte); + Slice* slice = reinterpret_cast(mem_ptr + 1); + char* data = reinterpret_cast(_mem_pool->allocate(storage_field_bytes)); + memory_copy(data, storage_ptr + sizeof(StringLengthType) + null_byte, storage_field_bytes); + slice->data = data; + slice->size = storage_field_bytes; } - size_t storage_field_bytes = - *reinterpret_cast(storage_ptr + null_byte); - Slice* slice = reinterpret_cast(mem_ptr + 1); - char* data = reinterpret_cast(_mem_pool->allocate(storage_field_bytes)); - memory_copy(data, storage_ptr + sizeof(StringLengthType) + null_byte, storage_field_bytes); - slice->data = data; - slice->size = storage_field_bytes; - mem_ptr += mem_row_bytes; storage_ptr += storage_row_bytes; } @@ -241,16 +239,14 @@ OLAPStatus MemIndex::load_segment(const char* file, size_t *current_num_rows_per // 2. copy length and content bool is_null = *reinterpret_cast(mem_ptr); - if (is_null) { - continue; + if (!is_null) { + Slice* slice = reinterpret_cast(mem_ptr + 1); + char* data = reinterpret_cast(_mem_pool->allocate(storage_field_bytes)); + memory_copy(data, storage_ptr + null_byte, storage_field_bytes); + slice->data = data; + slice->size = storage_field_bytes; } - Slice* slice = reinterpret_cast(mem_ptr + 1); - char* data = reinterpret_cast(_mem_pool->allocate(storage_field_bytes)); - memory_copy(data, storage_ptr + null_byte, storage_field_bytes); - slice->data = data; - slice->size = storage_field_bytes; - mem_ptr += mem_row_bytes; storage_ptr += storage_row_bytes; } @@ -263,12 +259,10 @@ OLAPStatus MemIndex::load_segment(const char* file, size_t *current_num_rows_per // 2. copy content bool is_null = *reinterpret_cast(mem_ptr); - if (is_null) { - continue; + if (!is_null) { + memory_copy(mem_ptr + 1, storage_ptr + null_byte, storage_field_bytes); } - memory_copy(mem_ptr + 1, storage_ptr + null_byte, storage_field_bytes); - mem_ptr += mem_row_bytes; storage_ptr += storage_row_bytes; }