Hello Dan Burkert, Alexey Serbin,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5120
to review the following change.
Change subject: KUDU-1749. Allow callers to disable SASL initialization
......................................................................
KUDU-1749. Allow callers to disable SASL initialization
The Kudu client makes use of SASL, as do other applications such as
Impala that use the client. This means that both Kudu and those
applications are expected to initialize SASL.
The sasl_*_init() functions themselves are safe to call multiple times
(they have no effect). However, sasl_set_mutex() is not safe to call
multiple times, nor is it safe to call after another library has already
begun using the SASL libraries.
IMPALA-4479 describes a crash in SaslMutexUnlock() which is likely
caused by this issue: Impala has initialized the SASL library with no
call to sasl_set_mutex(), and then during Kudu initialization we set the
mutex functions to our own. Then when we call sasl_dispose(), SASL tries
to acquire an internal mutex which was previously allocated to a garbage
value 0x1 by the built-in "fake" mutex implementation. This crashes when
we attempt to access it using our real mutex implementation.
The fix has a few parts:
- we have a new API which allows embedders to disable initialization of
SASL.
- we try to auto-detect the case in which the client has fully
initialized SASL including providing a mutex implementation. In that
case, we skip initialization but log a warning that the caller should
disable it explicitly.
- if we detect that it's initialized but no mutex implementation is
provided, we'll return an error on first usage of SASL. Similarly, if
they disable initialization but didn't do a proper job of
initialization, we detect it and return an error on usage.
The one sketchy thing about this patch is that the SASL API doesn't give
a 100% public way to determine its initialization state. However, it
does export a symbol which we can import that provides the necessary
information. This symbol is marked as an API, so we expect it to be
stable enough for usage, at least across the range of supported
operating systems.
Change-Id: I54d5ecfb2526852003f1ea249d1d2a8e6c0e91cd
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/sasl_rpc-test.cc
5 files changed, 179 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5120/1
--
To view, visit http://gerrit.cloudera.org:8080/5120
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I54d5ecfb2526852003f1ea249d1d2a8e6c0e91cd
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
