You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/03/01 02:56:58 UTC

[kudu-CR] Workaround a leak in OpenSSL 1.0.0

Hello Dan Burkert,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/6197

to review the following change.

Change subject: Workaround a leak in OpenSSL 1.0.0
......................................................................

Workaround a leak in OpenSSL 1.0.0

This fixes another leak seen occasionally as a flaky test on RHEL 6:

Direct leak of 56 byte(s) in 1 object(s) allocated from:
    #2 0x7f21e5fe037c in CRYPTO_malloc src/openssl/crypto/mem.c:306
    #3 0x7f21e607f45c in EVP_PKEY_new src/openssl/crypto/evp/p_lib.c:186
    #4 0x7f21e608ff33 in X509_PUBKEY_get src/openssl/crypto/asn1/x_pubkey.c:147
    #5 0x7f21e60b23ad in X509_get_pubkey src/openssl/crypto/x509/x509_cmp.c:292
    #6 0x7f21e60b623d in internal_verify src/openssl/crypto/x509/x509_vfy.c:1624
    #7 0x7f21e60b3b8b in X509_verify_cert src/openssl/crypto/x509/x509_vfy.c:372
    #8 0x7f21e5d5c096 in ssl_verify_cert_chain src/openssl/ssl/ssl_cert.c:535
    #9 0x7f21e5d34157 in ssl3_get_server_certificate src/openssl/ssl/s3_clnt.c:1047
    #10 0x7f21e5d3289f in ssl3_connect src/openssl/ssl/s3_clnt.c:310
    #11 0x7f21e5d5705b in SSL_connect src/openssl/ssl/ssl_lib.c:933
    #12 0x7f21e5d43340 in ssl23_get_server_hello src/openssl/ssl/s23_clnt.c:693
    #13 0x7f21e5d422b0 in ssl23_connect src/openssl/ssl/s23_clnt.c:222
    #14 0x7f21e5d59f46 in SSL_do_handshake src/openssl/ssl/ssl_lib.c:2380
    #15 0x7f21e6a028a7 in kudu::security::TlsHandshake::Continue(std::string const&, std::string*) src/kudu/security/tls_handshake.cc:92:12

The issue turned out to be an OpenSSL 1.0.0 bug that was since fixed. But,
since we build/test against OpenSSL 1.0.0 on el6, we hit the leak.

This patch has a workaround - see the included comment for details.

Before the patch, all_types-itest failed about 10% of the time in dist-test,
and now passes reliably.

Change-Id: I65fe981c523f6fe73b3668bfb2f30a95ebf3e759
---
M src/kudu/security/tls_context.cc
1 file changed, 16 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/6197/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I65fe981c523f6fe73b3668bfb2f30a95ebf3e759
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>

[kudu-CR] Workaround a leak in OpenSSL 1.0.0

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.

Change subject: Workaround a leak in OpenSSL 1.0.0
......................................................................


Workaround a leak in OpenSSL 1.0.0

This fixes another leak seen occasionally as a flaky test on RHEL 6:

Direct leak of 56 byte(s) in 1 object(s) allocated from:
    #2 0x7f21e5fe037c in CRYPTO_malloc src/openssl/crypto/mem.c:306
    #3 0x7f21e607f45c in EVP_PKEY_new src/openssl/crypto/evp/p_lib.c:186
    #4 0x7f21e608ff33 in X509_PUBKEY_get src/openssl/crypto/asn1/x_pubkey.c:147
    #5 0x7f21e60b23ad in X509_get_pubkey src/openssl/crypto/x509/x509_cmp.c:292
    #6 0x7f21e60b623d in internal_verify src/openssl/crypto/x509/x509_vfy.c:1624
    #7 0x7f21e60b3b8b in X509_verify_cert src/openssl/crypto/x509/x509_vfy.c:372
    #8 0x7f21e5d5c096 in ssl_verify_cert_chain src/openssl/ssl/ssl_cert.c:535
    #9 0x7f21e5d34157 in ssl3_get_server_certificate src/openssl/ssl/s3_clnt.c:1047
    #10 0x7f21e5d3289f in ssl3_connect src/openssl/ssl/s3_clnt.c:310
    #11 0x7f21e5d5705b in SSL_connect src/openssl/ssl/ssl_lib.c:933
    #12 0x7f21e5d43340 in ssl23_get_server_hello src/openssl/ssl/s23_clnt.c:693
    #13 0x7f21e5d422b0 in ssl23_connect src/openssl/ssl/s23_clnt.c:222
    #14 0x7f21e5d59f46 in SSL_do_handshake src/openssl/ssl/ssl_lib.c:2380
    #15 0x7f21e6a028a7 in kudu::security::TlsHandshake::Continue(std::string const&, std::string*) src/kudu/security/tls_handshake.cc:92:12

The issue turned out to be an OpenSSL 1.0.0 bug that was since fixed. But,
since we build/test against OpenSSL 1.0.0 on el6, we hit the leak.

This patch has a workaround - see the included comment for details.

Before the patch, all_types-itest failed about 10% of the time in dist-test,
and now passes reliably.

Change-Id: I65fe981c523f6fe73b3668bfb2f30a95ebf3e759
Reviewed-on: http://gerrit.cloudera.org:8080/6197
Reviewed-by: Dan Burkert <da...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/security/tls_context.cc
1 file changed, 16 insertions(+), 0 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I65fe981c523f6fe73b3668bfb2f30a95ebf3e759
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Workaround a leak in OpenSSL 1.0.0

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: Workaround a leak in OpenSSL 1.0.0
......................................................................


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65fe981c523f6fe73b3668bfb2f30a95ebf3e759
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No