You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by kw...@apache.org on 2018/10/02 22:25:10 UTC
[1/2] impala git commit: IMPALA-5031: null ptr errors in C calls in
BE tests
Repository: impala
Updated Branches:
refs/heads/master ea2809f5d -> e1d1b4f14
IMPALA-5031: null ptr errors in C calls in BE tests
This patch fixes all remaining UBSAN "null pointer passed as argument"
errors in the backend tests. These are undefined behavior according to
"7.1.4 Use of library functions" in the C99 standard (which is
included in C++14 in section [intro.refs]):
If an argument to a function has an invalid value (such as a value
outside the domain of the function, or a pointer outside the
address space of the program, or a null pointer, or a pointer to
non-modifiable storage when the corresponding parameter is not
const-qualified) or a type (after promotion) not expected by a
function with variable number of arguments, the behavior is
undefined.
The interesting parts of the backtraces for the errors fixed in this
patch are below:
exprs/string-functions-ir.cc:311:17: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:43:45: note: nonnull attribute specified here
#0 StringFunctions::Replace(impala_udf::FunctionContext*, impala_udf::StringVal const&, impala_udf::StringVal const&, impala_udf::StringVal const&) exprs/string-functions-ir.cc:311:5
#1 impala_udf::StringVal ScalarFnCall::InterpretEval<impala_udf::StringVal>(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:485:580
#2 ScalarFnCall::GetStringVal(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:599:44
#3 ScalarExprEvaluator::GetValue(ScalarExpr const&, TupleRow const*) exprs/scalar-expr-evaluator.cc:299:38
#4 ScalarExprEvaluator::GetValue(TupleRow const*) exprs/scalar-expr-evaluator.cc:250:10
#5 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, ScalarExprEvaluator* const*, MemPool*, StringValue**, int*, int*) runtime/tuple.cc:222:27
#6 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, vector<ScalarExprEvaluator*> const&, MemPool*, vector<StringValue*>*, int*) runtime/tuple.h:174:5
#7 UnionNode::MaterializeExprs(vector<ScalarExprEvaluator*> const&, TupleRow*, unsigned char*, RowBatch*) exec/union-node-ir.cc:29:14
#8 UnionNode::GetNextConst(RuntimeState*, RowBatch*) exec/union-node.cc:263:5
#9 UnionNode::GetNext(RuntimeState*, RowBatch*, bool*) exec/union-node.cc:296:45
#10 FragmentInstanceState::ExecInternal() runtime/fragment-instance-state.cc:310:59
#11 FragmentInstanceState::Exec() runtime/fragment-instance-state.cc:95:14
#12 QueryState::ExecFInstance(FragmentInstanceState*) runtime/query-state.cc:488:24
#13 QueryState::StartFInstances()::$_0::operator()() const runtime/query-state.cc:416:35
#20 thread_proxy (exprs/expr-test+0x55ca939)
exprs/string-functions-ir.cc:868:15: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:43:45: note: nonnull attribute specified here
#0 StringFunctions::ConcatWs(impala_udf::FunctionContext*, impala_udf::StringVal const&, int, impala_udf::StringVal const*) exprs/string-functions-ir.cc:868:3
#1 impala_udf::StringVal ScalarFnCall::InterpretEval<impala_udf::StringVal>(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:510:270
#2 ScalarFnCall::GetStringVal(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:599:44
#3 ScalarExprEvaluator::GetValue(ScalarExpr const&, TupleRow const*) exprs/scalar-expr-evaluator.cc:299:38
#4 ScalarExprEvaluator::GetValue(TupleRow const*) exprs/scalar-expr-evaluator.cc:250:10
#5 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, ScalarExprEvaluator* const*, MemPool*, StringValue**, int*, int*) runtime/tuple.cc:222:27
#6 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, vector<ScalarExprEvaluator*> const&, MemPool*, vector<StringValue*>*, int*) runtime/tuple.h:174:5
#7 UnionNode::MaterializeExprs(vector<ScalarExprEvaluator*> const&, TupleRow*, unsigned char*, RowBatch*) exec/union-node-ir.cc:29:14
#8 UnionNode::GetNextConst(RuntimeState*, RowBatch*) exec/union-node.cc:263:5
#9 UnionNode::GetNext(RuntimeState*, RowBatch*, bool*) exec/union-node.cc:296:45
#10 FragmentInstanceState::ExecInternal() runtime/fragment-instance-state.cc:310:59
#11 FragmentInstanceState::Exec() runtime/fragment-instance-state.cc:95:14
#12 QueryState::ExecFInstance(FragmentInstanceState*) runtime/query-state.cc:488:24
#13 QueryState::StartFInstances()::$_0::operator()() const runtime/query-state.cc:416:35
#20 thread_proxy (exprs/expr-test+0x55ca939)
exprs/string-functions-ir.cc:871:17: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:43:45: note: nonnull attribute specified here
#0 StringFunctions::ConcatWs(impala_udf::FunctionContext*, impala_udf::StringVal const&, int, impala_udf::StringVal const*) exprs/string-functions-ir.cc:871:5
#1 StringFunctions::Concat(impala_udf::FunctionContext*, int, impala_udf::StringVal const*) exprs/string-functions-ir.cc:843:10
#2 impala_udf::StringVal ScalarFnCall::InterpretEval<impala_udf::StringVal>(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:510:95
#3 ScalarFnCall::GetStringVal(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:599:44
#4 ScalarExprEvaluator::GetValue(ScalarExpr const&, TupleRow const*) exprs/scalar-expr-evaluator.cc:299:38
#5 ScalarExprEvaluator::GetValue(TupleRow const*) exprs/scalar-expr-evaluator.cc:250:10
#6 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, ScalarExprEvaluator* const*, MemPool*, StringValue**, int*, int*) runtime/tuple.cc:222:27
#7 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, vector<ScalarExprEvaluator*> const&, MemPool*, vector<StringValue*>*, int*) runtime/tuple.h:174:5
#8 UnionNode::MaterializeExprs(vector<ScalarExprEvaluator*> const&, TupleRow*, unsigned char*, RowBatch*) exec/union-node-ir.cc:29:14
#9 UnionNode::GetNextConst(RuntimeState*, RowBatch*) exec/union-node.cc:263:5
#10 UnionNode::GetNext(RuntimeState*, RowBatch*, bool*) exec/union-node.cc:296:45
#11 FragmentInstanceState::ExecInternal() runtime/fragment-instance-state.cc:310:59
#12 FragmentInstanceState::Exec() runtime/fragment-instance-state.cc:95:14
#13 QueryState::ExecFInstance(FragmentInstanceState*) runtime/query-state.cc:488:24
#14 QueryState::StartFInstances()::$_0::operator()() const runtime/query-state.cc:416:35
#21 thread_proxy (exprs/expr-test+0x55ca939)
exprs/string-functions-ir.cc:873:17: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:43:45: note: nonnull attribute specified here
#0 StringFunctions::ConcatWs(impala_udf::FunctionContext*, impala_udf::StringVal const&, int, impala_udf::StringVal const*) exprs/string-functions-ir.cc:873:5
#1 StringFunctions::Concat(impala_udf::FunctionContext*, int, impala_udf::StringVal const*) exprs/string-functions-ir.cc:843:10
#2 impala_udf::StringVal ScalarFnCall::InterpretEval<impala_udf::StringVal>(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:510:95
#3 ScalarFnCall::GetStringVal(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:599:44
#4 ScalarExprEvaluator::GetValue(ScalarExpr const&, TupleRow const*) exprs/scalar-expr-evaluator.cc:299:38
#5 ScalarExprEvaluator::GetValue(TupleRow const*) exprs/scalar-expr-evaluator.cc:250:10
#6 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, ScalarExprEvaluator* const*, MemPool*, StringValue**, int*, int*) runtime/tuple.cc:222:27
#7 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, vector<ScalarExprEvaluator*> const&, MemPool*, vector<StringValue*>*, int*) runtime/tuple.h:174:5
#8 UnionNode::MaterializeExprs(vector<ScalarExprEvaluator*> const&, TupleRow*, unsigned char*, RowBatch*) exec/union-node-ir.cc:29:14
#9 UnionNode::GetNextConst(RuntimeState*, RowBatch*) exec/union-node.cc:263:5
#10 UnionNode::GetNext(RuntimeState*, RowBatch*, bool*) exec/union-node.cc:296:45
#11 FragmentInstanceState::ExecInternal() runtime/fragment-instance-state.cc:310:59
#12 FragmentInstanceState::Exec() runtime/fragment-instance-state.cc:95:14
#13 QueryState::ExecFInstance(FragmentInstanceState*) runtime/query-state.cc:488:24
#14 QueryState::StartFInstances()::$_0::operator()() const runtime/query-state.cc:416:35
#21 thread_proxy (exprs/expr-test+0x55ca939)
runtime/raw-value.cc:159:27: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:43:45: note: nonnull attribute specified here
#0 RawValue::Write(void const*, void*, ColumnType const&, MemPool*) runtime/raw-value.cc:159:9
#1 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, ScalarExprEvaluator* const*, MemPool*, StringValue**, int*, int*) runtime/tuple.cc:225:7
#2 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, vector<ScalarExprEvaluator*> const&, MemPool*, vector<StringValue*>*, int*) runtime/tuple.h:174:5
#3 UnionNode::MaterializeExprs(vector<ScalarExprEvaluator*> const&, TupleRow*, unsigned char*, RowBatch*) exec/union-node-ir.cc:29:14
#4 UnionNode::GetNextConst(RuntimeState*, RowBatch*) exec/union-node.cc:263:5
#5 UnionNode::GetNext(RuntimeState*, RowBatch*, bool*) exec/union-node.cc:296:45
#6 FragmentInstanceState::ExecInternal() runtime/fragment-instance-state.cc:310:59
#7 FragmentInstanceState::Exec() runtime/fragment-instance-state.cc:95:14
#8 QueryState::ExecFInstance(FragmentInstanceState*) runtime/query-state.cc:488:24
#9 QueryState::StartFInstances()::$_0::operator()() const runtime/query-state.cc:416:35
#16 thread_proxy (exprs/expr-test+0x55ca939)
udf/udf.cc:521:24: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:43:45: note: nonnull attribute specified here
#0 impala_udf::StringVal::CopyFrom(impala_udf::FunctionContext*, unsigned char const*, unsigned long) udf/udf.cc:521:5
#1 AnyValUtil::FromBuffer(impala_udf::FunctionContext*, char const*, int) exprs/anyval-util.h:241:12
#2 StringFunctions::RegexpExtract(impala_udf::FunctionContext*, impala_udf::StringVal const&, impala_udf::StringVal const&, impala_udf::BigIntVal const&) exprs/string-functions-ir.cc:726:10
#3 impala_udf::StringVal ScalarFnCall::InterpretEval<impala_udf::StringVal>(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:485:580
#4 ScalarFnCall::GetStringVal(ScalarExprEvaluator*, TupleRow const*) const exprs/scalar-fn-call.cc:599:44
#5 ScalarExprEvaluator::GetValue(ScalarExpr const&, TupleRow const*) exprs/scalar-expr-evaluator.cc:299:38
#6 ScalarExprEvaluator::GetValue(TupleRow const*) exprs/scalar-expr-evaluator.cc:250:10
#7 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, ScalarExprEvaluator* const*, MemPool*, StringValue**, int*, int*) runtime/tuple.cc:222:27
#8 void Tuple::MaterializeExprs<false, false>(TupleRow*, TupleDescriptor const&, vector<ScalarExprEvaluator*> const&, MemPool*, vector<StringValue*>*, int*) runtime/tuple.h:174:5
#9 UnionNode::MaterializeExprs(vector<ScalarExprEvaluator*> const&, TupleRow*, unsigned char*, RowBatch*) exec/union-node-ir.cc:29:14
#10 UnionNode::GetNextConst(RuntimeState*, RowBatch*) exec/union-node.cc:263:5
#11 UnionNode::GetNext(RuntimeState*, RowBatch*, bool*) exec/union-node.cc:296:45
#12 FragmentInstanceState::ExecInternal() runtime/fragment-instance-state.cc:310:59
#13 FragmentInstanceState::Exec() runtime/fragment-instance-state.cc:95:14
#14 QueryState::ExecFInstance(FragmentInstanceState*) runtime/query-state.cc:488:24
#15 QueryState::StartFInstances()::$_0::operator()() const runtime/query-state.cc:416:35
#22 thread_proxy (exprs/expr-test+0x55ca939)
util/coding-util-test.cc:45:10: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:43:45: note: nonnull attribute specified here
#0 TestUrl(string const&, string const&, bool) util/coding-util-test.cc:45:3
#1 UrlCodingTest_BlankString_Test::TestBody() util/coding-util-test.cc:88:3
#2 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (util/coding-util-test+0x6630f42)
#8 main util/coding-util-test.cc:123:192
util/decompress-test.cc:126:261: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:66:58: note: nonnull attribute specified here
#0 DecompressorTest::CompressAndDecompress(Codec*, Codec*, long, unsigned char*) util/decompress-test.cc:126:254
#1 DecompressorTest::RunTest(THdfsCompression::type) util/decompress-test.cc:84:9
#2 DecompressorTest_Default_Test::TestBody() util/decompress-test.cc:373:3
#3 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (util/decompress-test+0x6642bb2)
#9 main util/decompress-test.cc:479:47
util/decompress-test.cc:148:261: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:66:58: note: nonnull attribute specified here
#0 DecompressorTest::CompressAndDecompress(Codec*, Codec*, long, unsigned char*) util/decompress-test.cc:148:254
#1 DecompressorTest::RunTest(THdfsCompression::type) util/decompress-test.cc:84:9
#2 DecompressorTest_Default_Test::TestBody() util/decompress-test.cc:373:3
#3 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (util/decompress-test+0x6642bb2)
#9 main util/decompress-test.cc:479:47
util/decompress-test.cc:269:261: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:66:58: note: nonnull attribute specified here
#0 DecompressorTest::CompressAndDecompressNoOutputAllocated(Codec*, Codec*, long, unsigned char*) util/decompress-test.cc:269:254
#1 DecompressorTest::RunTest(THdfsCompression::type) util/decompress-test.cc:71:7
#2 DecompressorTest_LZ4_Test::TestBody() util/decompress-test.cc:381:3
#3 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (util/decompress-test+0x6642bb2)
#9 main util/decompress-test.cc:479:47
util/decompress-test.cc:221:329: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:66:58: note: nonnull attribute specified here
#0 DecompressorTest::StreamingDecompress(Codec*, long, unsigned char*, long, unsigned char*, bool, long*) util/decompress-test.cc:221:322
#1 DecompressorTest::CompressAndStreamingDecompress(Codec*, Codec*, long, unsigned char*) util/decompress-test.cc:245:35
#2 DecompressorTest::RunTestStreaming(THdfsCompression::type) util/decompress-test.cc:104:5
#3 DecompressorTest_Gzip_Test::TestBody() util/decompress-test.cc:386:3
#4 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (util/decompress-test+0x6642bb2)
#10 main util/decompress-test.cc:479:47
util/streaming-sampler.h:55:22: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:43:45: note: nonnull attribute specified here
#0 StreamingSampler<long, 64>::StreamingSampler(int, vector<long> const&) util/streaming-sampler.h:55:5
#1 RuntimeProfile::TimeSeriesCounter::TimeSeriesCounter(string const&, TUnit::type, int, vector<long> const&) util/runtime-profile-counters.h:401:53
#2 RuntimeProfile::Update(vector<TRuntimeProfileNode> const&, int*) util/runtime-profile.cc:310:28
#3 RuntimeProfile::Update(TRuntimeProfileTree const&) util/runtime-profile.cc:245:3
#4 Coordinator::BackendState::InstanceStats::Update(TFragmentInstanceExecStatus const&, Coordinator::ExecSummary*, ProgressUpdater*) runtime/coordinator-backend-state.cc:473:13
#5 Coordinator::BackendState::ApplyExecStatusReport(TReportExecStatusParams const&, Coordinator::ExecSummary*, ProgressUpdater*) runtime/coordinator-backend-state.cc:286:21
#6 Coordinator::UpdateBackendExecStatus(TReportExecStatusParams const&) runtime/coordinator.cc:678:22
#7 ClientRequestState::UpdateBackendExecStatus(TReportExecStatusParams const&) service/client-request-state.cc:1253:18
#8 ImpalaServer::ReportExecStatus(TReportExecStatusResult&, TReportExecStatusParams const&) service/impala-server.cc:1343:18
#9 ImpalaInternalService::ReportExecStatus(TReportExecStatusResult&, TReportExecStatusParams const&) service/impala-internal-service.cc:87:19
#24 thread_proxy (exprs/expr-test+0x55ca939)
Change-Id: I317ccc99549744a26d65f3e07242079faad0355a
Reviewed-on: http://gerrit.cloudera.org:8080/11545
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/20bde289
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/20bde289
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/20bde289
Branch: refs/heads/master
Commit: 20bde289ebb6d1097b1afab7ad171498f4d164a7
Parents: ea2809f
Author: Jim Apple <jb...@apache.org>
Authored: Sat Sep 29 19:40:48 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Tue Oct 2 20:24:17 2018 +0000
----------------------------------------------------------------------
be/src/exprs/string-functions-ir.cc | 9 +++++----
be/src/runtime/raw-value.cc | 3 ++-
be/src/udf/udf.cc | 2 +-
be/src/util/coding-util-test.cc | 3 ++-
be/src/util/decompress-test.cc | 10 ++++++----
be/src/util/streaming-sampler.h | 3 ++-
be/src/util/ubsan.h | 9 +++++++++
7 files changed, 27 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/20bde289/be/src/exprs/string-functions-ir.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/string-functions-ir.cc b/be/src/exprs/string-functions-ir.cc
index 01947ed..f5e8213 100644
--- a/be/src/exprs/string-functions-ir.cc
+++ b/be/src/exprs/string-functions-ir.cc
@@ -31,6 +31,7 @@
#include "runtime/tuple-row.h"
#include "util/bit-util.h"
#include "util/coding-util.h"
+#include "util/ubsan.h"
#include "util/url-parser.h"
#include "common/names.h"
@@ -308,7 +309,7 @@ StringVal StringFunctions::Replace(FunctionContext* context, const StringVal& st
// Copy in replacement - always safe since we always leave room for one more replace
DCHECK_LE(ptr - result.ptr + replace.len, buffer_space);
- memcpy(ptr, replace.ptr, replace.len);
+ Ubsan::MemCpy(ptr, replace.ptr, replace.len);
ptr += replace.len;
// Don't want to re-match within already replaced pattern
@@ -865,12 +866,12 @@ StringVal StringFunctions::ConcatWs(FunctionContext* context, const StringVal& s
// Loop again to append the data.
uint8_t* ptr = result.ptr;
- memcpy(ptr, strs[0].ptr, strs[0].len);
+ Ubsan::MemCpy(ptr, strs[0].ptr, strs[0].len);
ptr += strs[0].len;
for (int32_t i = 1; i < num_children; ++i) {
- memcpy(ptr, sep.ptr, sep.len);
+ Ubsan::MemCpy(ptr, sep.ptr, sep.len);
ptr += sep.len;
- memcpy(ptr, strs[i].ptr, strs[i].len);
+ Ubsan::MemCpy(ptr, strs[i].ptr, strs[i].len);
ptr += strs[i].len;
}
return result;
http://git-wip-us.apache.org/repos/asf/impala/blob/20bde289/be/src/runtime/raw-value.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/raw-value.cc b/be/src/runtime/raw-value.cc
index d030460..b5de7bc 100644
--- a/be/src/runtime/raw-value.cc
+++ b/be/src/runtime/raw-value.cc
@@ -22,6 +22,7 @@
#include "runtime/raw-value.inline.h"
#include "runtime/string-value.inline.h"
#include "runtime/tuple.h"
+#include "util/ubsan.h"
#include "common/names.h"
@@ -156,7 +157,7 @@ void RawValue::Write(const void* value, void* dst, const ColumnType& type,
// to reflect this change as well (the codegen'd Allocate() call is actually
// generated in CodegenAnyVal::StoreToNativePtr()).
dest->ptr = reinterpret_cast<char*>(pool->Allocate(dest->len));
- memcpy(dest->ptr, src->ptr, dest->len);
+ Ubsan::MemCpy(dest->ptr, src->ptr, dest->len);
} else {
dest->ptr = src->ptr;
}
http://git-wip-us.apache.org/repos/asf/impala/blob/20bde289/be/src/udf/udf.cc
----------------------------------------------------------------------
diff --git a/be/src/udf/udf.cc b/be/src/udf/udf.cc
index 4ed3cc2..5edfec3 100644
--- a/be/src/udf/udf.cc
+++ b/be/src/udf/udf.cc
@@ -518,7 +518,7 @@ StringVal::StringVal(FunctionContext* context, int str_len) noexcept : len(str_l
StringVal StringVal::CopyFrom(FunctionContext* ctx, const uint8_t* buf, size_t len) noexcept {
StringVal result(ctx, len);
if (LIKELY(!result.is_null)) {
- memcpy(result.ptr, buf, len);
+ std::copy(buf, buf + len, result.ptr);
}
return result;
}
http://git-wip-us.apache.org/repos/asf/impala/blob/20bde289/be/src/util/coding-util-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/coding-util-test.cc b/be/src/util/coding-util-test.cc
index be479a0..402f0fc 100644
--- a/be/src/util/coding-util-test.cc
+++ b/be/src/util/coding-util-test.cc
@@ -22,6 +22,7 @@
#include "common/logging.h"
#include "testutil/gtest-util.h"
#include "util/coding-util.h"
+#include "util/ubsan.h"
#include "common/names.h"
@@ -42,7 +43,7 @@ void TestUrl(const string& input, const string& expected_encoded, bool hive_comp
// Convert string to vector and try that also
vector<uint8_t> input_vector;
input_vector.resize(input.size());
- memcpy(input_vector.data(), input.c_str(), input.size());
+ Ubsan::MemCpy(input_vector.data(), input.c_str(), input.size());
string intermediate2;
UrlEncode(input_vector, &intermediate2, hive_compat);
EXPECT_EQ(intermediate, intermediate2);
http://git-wip-us.apache.org/repos/asf/impala/blob/20bde289/be/src/util/decompress-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/decompress-test.cc b/be/src/util/decompress-test.cc
index 4d5f8ef..ce50884 100644
--- a/be/src/util/decompress-test.cc
+++ b/be/src/util/decompress-test.cc
@@ -26,6 +26,7 @@
#include "testutil/gtest-util.h"
#include "util/decompress.h"
#include "util/compress.h"
+#include "util/ubsan.h"
#include "common/names.h"
@@ -123,7 +124,7 @@ class DecompressorTest : public ::testing::Test {
compressed, &output_len, &output));
EXPECT_EQ(output_len, input_len);
- EXPECT_EQ(memcmp(input, output, input_len), 0);
+ EXPECT_EQ(Ubsan::MemCmp(input, output, input_len), 0);
// Preallocated output buffers
int64_t max_compressed_length = compressor->MaxOutputLen(input_len, input);
@@ -145,7 +146,7 @@ class DecompressorTest : public ::testing::Test {
&output_len, &output));
EXPECT_EQ(output_len, input_len);
- EXPECT_EQ(memcmp(input, output, input_len), 0);
+ EXPECT_EQ(Ubsan::MemCmp(input, output, input_len), 0);
}
// Test the behavior when the decompressor is given too little / too much space.
@@ -218,7 +219,8 @@ class DecompressorTest : public ::testing::Test {
int64_t compressed_bytes_read = 0;
RETURN_IF_ERROR(decompressor->ProcessBlockStreaming(compressed_bytes_remaining,
compressed_input, &compressed_bytes_read, &output_len, &output, &stream_end));
- EXPECT_EQ(memcmp(uncompressed_input + decompressed_len, output, output_len), 0);
+ EXPECT_EQ(
+ Ubsan::MemCmp(uncompressed_input + decompressed_len, output, output_len), 0);
decompressed_len += output_len;
EXPECT_LE(decompressed_len, uncompressed_len);
compressed_input = compressed_input + compressed_bytes_read;
@@ -266,7 +268,7 @@ class DecompressorTest : public ::testing::Test {
&output_len, &output));
EXPECT_EQ(output_len, input_len);
- EXPECT_EQ(memcmp(input, output, input_len), 0);
+ EXPECT_EQ(Ubsan::MemCmp(input, output, input_len), 0);
}
void RunTestMultiStreamDecompressing(THdfsCompression::type format) {
http://git-wip-us.apache.org/repos/asf/impala/blob/20bde289/be/src/util/streaming-sampler.h
----------------------------------------------------------------------
diff --git a/be/src/util/streaming-sampler.h b/be/src/util/streaming-sampler.h
index 3d254da..dfa3dab 100644
--- a/be/src/util/streaming-sampler.h
+++ b/be/src/util/streaming-sampler.h
@@ -23,6 +23,7 @@
#include <boost/thread/lock_guard.hpp>
#include "util/spinlock.h"
+#include "util/ubsan.h"
namespace impala {
@@ -52,7 +53,7 @@ class StreamingSampler {
current_sample_count_(0),
current_sample_total_time_(0) {
DCHECK_LE(samples_collected_, MAX_SAMPLES);
- memcpy(samples_, initial_samples.data(), sizeof(T) * samples_collected_);
+ Ubsan::MemCpy(samples_, initial_samples.data(), sizeof(T) * samples_collected_);
}
/// Add a sample to the sampler. 'ms' is the time elapsed since the last time this
http://git-wip-us.apache.org/repos/asf/impala/blob/20bde289/be/src/util/ubsan.h
----------------------------------------------------------------------
diff --git a/be/src/util/ubsan.h b/be/src/util/ubsan.h
index da8b843..d1f45eb 100644
--- a/be/src/util/ubsan.h
+++ b/be/src/util/ubsan.h
@@ -23,6 +23,8 @@
#include <cstring>
+#include "common/logging.h"
+
class Ubsan {
public:
static void* MemSet(void* s, int c, size_t n) {
@@ -39,6 +41,13 @@ class Ubsan {
}
return std::memcpy(dest, src, n);
}
+ static int MemCmp(const void *s1, const void *s2, size_t n) {
+ if (s1 == nullptr || s2 == nullptr) {
+ DCHECK_EQ(n, 0);
+ return 0;
+ }
+ return std::memcmp(s1, s2, n);
+ }
};
#endif // UTIL_UBSAN_H_
[2/2] impala git commit: IMPALA-7585: Always set user credentials
after creating RPC proxy
Posted by kw...@apache.org.
IMPALA-7585: Always set user credentials after creating RPC proxy
kudu::rpc::Proxy() ctor may fail in GetLoggedInUser() for various reasons
(e.g. missing certain libraries). This resulted in an empty username being
used in kudu::rpc::ConnectionId. With plaintext SASL (e.g. in an insecure
Impala cluster), this may result in the following error during connection
negotiation:
Not authorized: Client connection negotiation failed: client connection to 127.0.0.1:27000: SASL(-1): generic failure: All-whitespace username.
In Impala, we don't consider failing GetLoggedInUser() a fatal error.
This change fixes the issue above by always explicitly setting the
username after creating the proxy. The username is "impala". Please
note that this username is not really used anywhere for authorization
for RPC services. Authorization is only done when authentication is
enabled with Kerberos. With Kerberos enabled, the username is derived
from the Kerberos principal instead of the user credentials set in
the ConnectionId. It's there mostly to satisfy the SASL plaintext case.
rpc-mgr-test has been updated to test for this string when Kerberos is
disabled.
Testing done: core test; rpc-mgr-test; rpc-mgr-kerberized-test
Change-Id: I75059f55bcdb8f95916610100cad4d8280daf3f6
Reviewed-on: http://gerrit.cloudera.org:8080/11477
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/e1d1b4f1
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/e1d1b4f1
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/e1d1b4f1
Branch: refs/heads/master
Commit: e1d1b4f14f1a2d8cab378b419eed3c4e4590a311
Parents: 20bde28
Author: Michael Ho <kw...@cloudera.com>
Authored: Tue Sep 18 23:59:01 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Tue Oct 2 21:55:44 2018 +0000
----------------------------------------------------------------------
be/src/exec/kudu-util.h | 1 +
be/src/rpc/rpc-mgr-test.h | 7 ++++++-
be/src/rpc/rpc-mgr.inline.h | 8 ++++++++
3 files changed, 15 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/e1d1b4f1/be/src/exec/kudu-util.h
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-util.h b/be/src/exec/kudu-util.h
index 8ed25a9..37755bd 100644
--- a/be/src/exec/kudu-util.h
+++ b/be/src/exec/kudu-util.h
@@ -21,6 +21,7 @@
// TODO: Remove when toolchain callbacks.h properly defines ::tm.
struct tm;
+#include <gutil/strings/substitute.h>
#include <kudu/client/callbacks.h>
#include <kudu/client/client.h>
#include <kudu/client/value.h>
http://git-wip-us.apache.org/repos/asf/impala/blob/e1d1b4f1/be/src/rpc/rpc-mgr-test.h
----------------------------------------------------------------------
diff --git a/be/src/rpc/rpc-mgr-test.h b/be/src/rpc/rpc-mgr-test.h
index bd2ea96..d95df36 100644
--- a/be/src/rpc/rpc-mgr-test.h
+++ b/be/src/rpc/rpc-mgr-test.h
@@ -31,6 +31,7 @@
#include "runtime/mem-tracker.h"
#include "testutil/gtest-util.h"
#include "testutil/scoped-flag-setter.h"
+#include "util/auth-util.h"
#include "util/network-util.h"
#include "util/openssl-util.h"
#include "util/test-info.h"
@@ -175,7 +176,11 @@ class PingServiceImpl : public PingServiceIf {
virtual bool Authorize(const google::protobuf::Message* req,
google::protobuf::Message* resp, RpcContext* context) override {
- return rpc_mgr_->Authorize("PingService", context, mem_tracker());
+ if (!IsKerberosEnabled()) {
+ return context->remote_user().username() == "impala";
+ } else {
+ return rpc_mgr_->Authorize("PingService", context, mem_tracker());
+ }
}
virtual void Ping(const PingRequestPB* request, PingResponsePB* response, RpcContext*
http://git-wip-us.apache.org/repos/asf/impala/blob/e1d1b4f1/be/src/rpc/rpc-mgr.inline.h
----------------------------------------------------------------------
diff --git a/be/src/rpc/rpc-mgr.inline.h b/be/src/rpc/rpc-mgr.inline.h
index 934b28b..9a1fc7e 100644
--- a/be/src/rpc/rpc-mgr.inline.h
+++ b/be/src/rpc/rpc-mgr.inline.h
@@ -24,6 +24,7 @@
#include "kudu/rpc/messenger.h"
#include "kudu/rpc/rpc_header.pb.h"
#include "kudu/rpc/service_pool.h"
+#include "kudu/rpc/user_credentials.h"
#include "util/network-util.h"
namespace impala {
@@ -38,6 +39,13 @@ Status RpcMgr::GetProxy(const TNetworkAddress& address, const std::string& hostn
kudu::Sockaddr sockaddr;
RETURN_IF_ERROR(TNetworkAddressToSockaddr(address, &sockaddr));
proxy->reset(new P(messenger_, sockaddr, hostname));
+
+ // Always set the user credentials as Proxy ctor may fail in GetLoggedInUser().
+ // An empty user name will result in SASL failure. See IMPALA-7585.
+ kudu::rpc::UserCredentials creds;
+ creds.set_real_user("impala");
+ (*proxy)->set_user_credentials(creds);
+
return Status::OK();
}