incubator-impala git commit: IMPALA-6126: ASAN detects heap-use-after-free in thrift-server-test

[sa...@apache.org]

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 <sa...@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/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 <sa...@apache.org>
Authored: Sun Oct 29 21:54:52 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
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(&current_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();
+}