Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/12545 )

Change subject: KUDU-2706: Work around the lack of thread safety in 
krb5_parse_name()
......................................................................

KUDU-2706: Work around the lack of thread safety in krb5_parse_name()

krb5_init_context() sets the field 'default_realm' in a krb5_context
object to 0. Upon first call to krb5_parse_name() with a principal
without realm specified (e.g. foo/bar), 'default_realm' in the
krb5_context object is lazily initialized.

When more than one negotiation threads are configured, it's possible
for multiple threads to call CanonicalizeKrb5Principal() in parallel.
CanonicalizeKrb5Principal() in turn calls krb5_parse_name(g_krb5_ctx, ...)
with no lock held. In addition, krb5_parse_name() is not thread safe as
it lazily initializes 'context->default_realm' without holding lock.
Consequently, 'g_krb5_ctx' which is shared and not supposed to be modified
after initialization may be inadvertently modified concurrently by multiple
threads, leading to crashes (e.g. double free) or errors.

This change works around the problem by initializing 'g_krb5_ctx->default_realm'
once in InitKrb5Ctx() by calling krb5_get_default_realm().

TODO: Fix unsafe sharing of 'g_krb5_ctx'. According to Kerberos documentation
(https://github.com/krb5/krb5/blob/master/doc/threads.txt), any use of 
krb5_context
must be confined to one thread at a time by the application code. The current
sharing of 'g_krb5_ctx' between threads without synchronization is in fact 
unsafe.

Change-Id: I1bf9224516e2996f51f319088179727f76741ebe
Reviewed-on: http://gerrit.cloudera.org:8080/12545
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Kudu Jenkins
---
M src/kudu/security/init.cc
1 file changed, 12 insertions(+), 0 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/12545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1bf9224516e2996f51f319088179727f76741ebe
Gerrit-Change-Number: 12545
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Ho <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <[email protected]>

Reply via email to