Repository: incubator-impala Updated Branches: refs/heads/master d1594298e -> 1b0852c9d
IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test A recent patch for IMPALA-5129 introduced a use-after-free bug in thrift-server-test. It is fixed in this patch. Change-Id: I2cd434757de2cd384def5b360a479e51812cccca Reviewed-on: http://gerrit.cloudera.org:8080/8412 Reviewed-by: Sailesh Mukil <[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/1b0852c9 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/1b0852c9 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/1b0852c9 Branch: refs/heads/master Commit: 1b0852c9d65c62b21df7bf7a9b1398052d5a1396 Parents: d159429 Author: Sailesh Mukil <[email protected]> Authored: Sun Oct 29 21:54:52 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Oct 31 21:14:57 2017 +0000 ---------------------------------------------------------------------- be/src/rpc/auth-provider.h | 4 ++++ be/src/rpc/thrift-server-test.cc | 18 +++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1b0852c9/be/src/rpc/auth-provider.h ---------------------------------------------------------------------- diff --git a/be/src/rpc/auth-provider.h b/be/src/rpc/auth-provider.h index 63d2fe1..f430f3e 100644 --- a/be/src/rpc/auth-provider.h +++ b/be/src/rpc/auth-provider.h @@ -179,6 +179,10 @@ class NoAuthProvider : public AuthProvider { /// The first entry point to the authentication subsystem. Performs initialization /// of Sasl, the global AuthManager, and the two authentication providers. Appname /// should generally be argv[0]. +/// TODO: Calling InitAuth() more than once is not an issue, however, calling InitAuth() +/// more than once with a different 'appname' can lead to undefined behavior. Also, +/// 'appname' must live as long as the process does since the cyrus-sasl library holds a +/// reference to it. Status InitAuth(const std::string& appname); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1b0852c9/be/src/rpc/thrift-server-test.cc ---------------------------------------------------------------------- diff --git a/be/src/rpc/thrift-server-test.cc b/be/src/rpc/thrift-server-test.cc index c1ad4fc..77469f0 100644 --- a/be/src/rpc/thrift-server-test.cc +++ b/be/src/rpc/thrift-server-test.cc @@ -106,6 +106,10 @@ template <class T> class ThriftTestBase : public T { virtual void TearDown() {} }; +// The path of the current executable file that is required for passing into the SASL +// library as the 'application name'. +string current_executable_path; + // This class allows us to run all the tests that derive from this in the modes enumerated // in 'KerberosSwitch'. // If the mode is USE_KUDU_KERBEROS or USE_IMPALA_KERBEROS, the MiniKdc which is a wrapper @@ -135,8 +139,9 @@ class ThriftParamsTest : public ThriftTestBase<testing::TestWithParam<KerberosSw FLAGS_principal = Substitute("$0@$1", spn, realm); } - string current_executable_path; - KUDU_ASSERT_OK(kudu::Env::Default()->GetExecutablePath(¤t_executable_path)); + + // Make sure that we have a valid string in the 'current_executable_path'. + ASSERT_FALSE(current_executable_path.empty()); ASSERT_OK(InitAuth(current_executable_path)); } @@ -617,4 +622,11 @@ TEST(NoPasswordPemFile, BadServerCertificate) { }, TSSLException); } -IMPALA_TEST_MAIN(); +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + impala::InitCommonRuntime(argc, argv, false, impala::TestInfo::BE_TEST); + + // Fill in the path of the current binary for use by the tests. + current_executable_path = argv[0]; + return RUN_ALL_TESTS(); +}
