Memory leak detected by AddressSanitizer

[nataraj-hates-MS-for-stealing-github]

Hi! I decided to test perl modules built with AddressSanitizer. I did not go very far, as I found out that Net::SSLeay fail tests when build with ASan. And it is needed for sending test reports :-)

How to reproduce:

1. Build perl with ASan. With perlbrew it would be like this:

perlbrew install -Dcc=clang-16 -Accflags="-fsanitize=address" -Aldflags="-fsanitize=address" -DEBUGGING=both  -v perl-5.38.2 --as=5.38.2-asan-debug

2. Using that perl install all Net::SSLeay dependencies from CPAN, and build Net::SSLeay itself. You will see that tests fails.

3. Try to run following script using Net::SSLeay build in p.2:

use strict;
use Symbol qw(gensym);
use Net::SSLeay::Handle;

my @uris = ('ya.ru', 'google.com');
#my @uris = ('google.com');

for my $uri (@uris)
{
  my $ssl = gensym();
  tie(*$ssl, "Net::SSLeay::Handle", $uri, 443);
  print $ssl "GET / HTTP/1.0\r\n\r\n";

  my $response = do { local $/ = undef; <$ssl> };
}

You will see that this script will cause memory leak reports if you've read from two sockets in a row. And will behave nice if there is only one socket is used (you need to use commented alternatice @uris line). The key to the problem is reading from socket. If you just write to it without reading, it will not leak to leaks.

I have not dig into that problem yet, I've just find solid way to reproduce it. I suspect that there some kind of OPENSSL_init_ssl is called twice, or something like that.

You know that code better then me, you have better chances to find the origin of the problem. But I will give it another try soon, I hope

[nataraj-hates-MS-for-stealing-github]

P.S.: OpenSSL normally works well with AddressSanitizer as far as I've looked in the net, so there should be somehting wrong with Net::SSLeay side

[h-vn]

Here's a quick fix: add close $ssl; at the end of the loop after the line starting with my $response .... This triggers a call to sub CLOSE in Handle.pm which frees OpenSSL's SSL and SSL_CTX objects. Without the close call I see a long list of leak information with a summary like this: SUMMARY: AddressSanitizer: 15299 byte(s) leaked in 163 allocation(s).

Your example of enabling leak detection is very useful. This is how I compiled Perl on a macOS with a Clang compiler from a homebrew installation. It's the same command as you used, but with a direct path pointing to cc:

% perlbrew install -Dcc=/usr/local/Cellar/llvm/17.0.6/bin/clang  -Accflags="-fsanitize=address" -Aldflags="-fsanitize=address" -DEBUGGING=both  -v perl-5.38.2 --as=5.38.2-asan-debug

I then compiled Net::SSLeay as follows. Setting OPTIMIZE is not required, but I thought it may be useful to turn off optimisation and add debug information with -g.

% perl Makefile.PL CC=/usr/local/Cellar/llvm/17.0.6/bin/clang OPTIMIZE=-g
[picks up OpenSSL 3.2.0 from homebrew installation]

To get useful reports I had to first set MallocNanoZone environment variable, which appears to be macOS specific, to avoid a warning from the clib or thereabouts:

% export MallocNanoZone=0

Otherwise this is logged: perl(21439,0x7ff84a197b80) malloc: nano zone abandoned due to inability to reserve vm space.

I then run Perl like shown below:

% ASAN_OPTIONS=detect_leaks=1 LSAN_OPTIONS=suppressions=suppr.txt  perl -I blib/lib -I blib/arch t/local/45_exporter.t
...
[cut test results]
-----------------------------------------------------
Suppressions used:
  count      bytes template
     16       2976 getaddrinfo
      6       1792 Perl_pp_fork
      2        168 *_os_trace*
-----------------------------------------------------

File suppr.txt looks as shown below. It seems that getaddrinfo and fork allocate memory in a way that looks like a leak:

% cat suppr.txt 
leak:getaddrinfo
leak:Perl_pp_fork

A number of tests appear to skip calling Net::SSLeay::free($ssl);, Net::SSLeay::CTX_free($ctx); and other functions needed for a correct cleanup. For the sake not being bad examples I can take a look at those at least.

[nataraj-hates-MS-for-stealing-github]

Thank you for the answer.

File suppr.txt looks as shown below. It seems that getaddrinfo and fork allocate memory in a way that looks like a leak:

% cat suppr.txt
leak:getaddrinfo
leak:Perl_pp_fork

This sounds strange for me, because I've successfully built perl itself, and all modules that go with it, using ASan sanitizer, and all tests have successfully passed. So I would doubt that such generic thing as fork or getaddrinfo has some ASan related bug in it.

But we can fix one thing in a time, so this is not critical...

I then compiled Net::SSLeay as follows. Setting OPTIMIZE is not required, but I thought it may be useful to turn off optimisation and add debug information with -g.

I am still not good this perl building harness. I've googled that -DEBUGGING=both should do the trick. But thank you, I will check that out.

For the sake not being bad examples I can take a look at those at least.

Please let me know about your progress... I am going to have vacations in a week, and I planed to dedicate it to perl+ASan related issues, including looking into this module again. I have some openssl related experience. So keep me informed, so we do not do same things in the same time :-)

[nataraj-hates-MS-for-stealing-github]

I've suggested a fix for this problem: #473

This does not solve all ASan problems thought.

Do not hurry with creating new release, I will continue dealing with these problems soon, and might provide new patches.

[nataraj-hates-MS-for-stealing-github]

% perl Makefile.PL CC=/usr/local/Cellar/llvm/17.0.6/bin/clang OPTIMIZE=-g

I've checked with my perl, you do not need to add extra params here, (at least in linux)

-g is provided by -DEBUGGING=both option given to perlbrew. And all build options when you build the module will be the same as they were when perl have been built. So nothing extra is needed. And if something is needed it should be added to perlbrew options....

[nataraj-hates-MS-for-stealing-github]

Next question:
Leak report from t/local/11_read.t can be fixed by adding explicit Net::SSLeay::CTX_free

diff --git a/t/local/11_read.t b/t/local/11_read.t
index bab0ec0..b185d40 100644
--- a/t/local/11_read.t
+++ b/t/local/11_read.t
@@ -61,6 +61,7 @@ sub server
            Net::SSLeay::write($ssl, $msg);
            Net::SSLeay::shutdown($ssl);
            Net::SSLeay::free($ssl);
+           Net::SSLeay::CTX_free($ctx);
            close($cl) || die("client close: $!");
        }
        $server->close() || die("server listen socket close: $!");
@@ -93,6 +94,7 @@ sub client
 
        Net::SSLeay::shutdown($ssl);
        Net::SSLeay::free($ssl);
+       Net::SSLeay::CTX_free($ctx);
        close($cl) || die("client close: $!");
     }
     $server->close() || die("client listen socket close: $!");

My question is: do we need some automatic free when $ctx goes out of scope, or we consider it just perl version of C functions, and calling Net::SSLeay::CTX_free is mandatory as it is mandatory in C. So it is either automatic free, or mention in documentation. What would we choose?