AddressSanitizer found memory leak in ssl_security_connector.cc @ try_replace_server_handshak()

[jesus.vel...@gmail.com]

Working with gRPC version v1.18.X I found that ASAN was complaining:

Direct leak of 888 byte(s) in 1 object(s) allocated from:
    #0 0x7fdc89b6ddc2 in __interceptor_malloc /home/andrey/toolchain-builder/.build/x86_64-ocivcn-linux-gnu/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:62
    #1 0x10e6e3f in gpr_malloc external/grpc/src/core/lib/gpr/alloc.cc:57
    #2 0x10e926a in gpr_strdup external/grpc/src/core/lib/gpr/string.cc:46
    #3 0xde8aea in grpc_convert_grpc_to_tsi_cert_pairs(grpc_ssl_pem_key_cert_pair const*, unsigned long) external/grpc/src/core/lib/security/credentials/ssl/ssl_credentials.cc:177
    #4 0xdee0fe in try_replace_server_handshaker_factory external/grpc/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc:126
    #5 0xdee0fe in try_fetch_ssl_server_credentials external/grpc/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc:170
    #6 0xdee428 in ssl_server_add_handshakers external/grpc/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc:191
    #7 0xdebe25 in grpc_server_security_connector_add_handshakers(grpc_server_security_connector*, grpc_pollset_set*, grpc_handshake_manager*) external/grpc/src/core/lib/security/security_connector/security_connector.cc:57
    #8 0xdf077f in server_handshaker_factory_add_handshakers external/grpc/src/core/lib/security/transport/security_handshaker.cc:491
    #9 0x10bc606 in grpc_handshaker_factory_add_handshakers(grpc_handshaker_factory*, grpc_channel_args const*, grpc_pollset_set*, grpc_handshake_manager*) external/grpc/src/core/lib/channel/handshaker_factory.cc:31
    #10 0x10302d2 in grpc_handshaker_factory_list_add_handshakers external/grpc/src/core/lib/channel/handshaker_registry.cc:57
    #11 0x10302d2 in grpc_handshakers_add(grpc_handshaker_type, grpc_channel_args const*, grpc_pollset_set*, grpc_handshake_manager*) external/grpc/src/core/lib/channel/handshaker_registry.cc:98
    #12 0xfc64f4 in on_accept external/grpc/src/core/ext/transport/chttp2/server/chttp2_server.cc:230
    #13 0x1069e9a in on_read external/grpc/src/core/lib/iomgr/tcp_server_posix.cc:245
    #14 0x10482c2 in exec_ctx_run external/grpc/src/core/lib/iomgr/exec_ctx.cc:40
    #15 0x10485a1 in grpc_core::ExecCtx::Flush() external/grpc/src/core/lib/iomgr/exec_ctx.cc:134
    #16 0x10ca008 in pollset_work external/grpc/src/core/lib/iomgr/ev_epollex_linux.cc:1199
    #17 0x1046802 in pollset_work external/grpc/src/core/lib/iomgr/ev_posix.cc:317
    #18 0x104e5e7 in grpc_pollset_work(grpc_pollset*, grpc_pollset_worker**, long) external/grpc/src/core/lib/iomgr/pollset.cc:48

Taking a look to the code, the problem is that cert_pairs is freed but not individual cert_chains and private_keys. This is my current solution:

index 20a9533dd1..0055c31f9c 100644
--- a/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc
+++ b/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc
@@ -133,6 +133,11 @@ static bool try_replace_server_handshaker_factory(
           server_creds->config.client_certificate_request),
       grpc_get_ssl_cipher_suites(), alpn_protocol_strings,
       static_cast<uint16_t>(num_alpn_protocols), &new_handshaker_factory);
+
+  for (size_t i = 0; i < config->num_key_cert_pairs; i++) {
+      gpr_free((void *)cert_pairs[i].private_key);
+      gpr_free((void *)cert_pairs[i].cert_chain);
+  }
   gpr_free(cert_pairs);
   gpr_free((void*)alpn_protocol_strings);

ASAN does not complain anymore.

[Jihun Cho]

Thanks for the suggestion. Can you create a pull request?