[Vm-dev] Memory leak in the SqueakSSL plugin on unix

[David T. Lewis]

Hi Levente,

I opened a Mantis issue to make sure this does not get overlooked:

  http://bugs.squeak.org/view.php?id=7793

Do you now have an updated sqUnixOpenSSL.c that fixes the memory leak
problem? If so can you please post the file here (or attach it to the Mantis
report)? I understand the changes from your original message but I'm not
clear if more work needs to be done related to "the BIO_free_all calls are
not needed in general".

The sqUnixOpenSSL.c file is identical on trunk and oscog, so we can apply
your fix to both branches.

Thanks!
Dave


On Thu, Oct 17, 2013 at 11:50:32PM +0200, Levente Uzonyi wrote:
> 
> I tried to set up a build environment to be able to build a functional 
> SqueakSSL plugin and test this idea. The only barrier was that the plugin 
> is not linked to libssl nor to libcrypto, so it won't do anything. You can 
> use ldd to check if this is the case with your plugin too:
> 
> $ ldd SqueakSSL
> 	linux-gate.so.1 =>  (0xf7763000)
> 	libc.so.6 => /lib/i386-linux-gnu/libc.so.6 (0xf7595000)
> 	/lib/ld-linux.so.2 (0xf7764000)
> 
> This is currently the case with Eliot's build.
> To make the plugin work I added -lssl and -lcrypto manually to the 
> Makefile. I have no idea how to add it to autoconf, so someone else will 
> have to do that. With those flags ldd will show something like this:
> 
> $ ldd SqueakSSL
> 	linux-gate.so.1 =>  (0xf77c0000)
> 	libssl.so.1.0.0 => /lib/i386-linux-gnu/libssl.so.1.0.0 (0xf7760000)
> 	libcrypto.so.1.0.0 => /lib/i386-linux-gnu/libcrypto.so.1.0.0 
> 	(0xf75b5000)
> 	libc.so.6 => /lib/i386-linux-gnu/libc.so.6 (0xf740b000)
> 	libdl.so.2 => /lib/i386-linux-gnu/libdl.so.2 (0xf7406000)
> 	libz.so.1 => /lib/i386-linux-gnu/libz.so.1 (0xf73f0000)
> 	/lib/ld-linux.so.2 (0xf77c1000)
> 
> It quickly turned out that the BIO_free_all calls are not needed in 
> general, only when the ssl structure doesn't get allocated, because 
> SSL_free frees both bioRead and bioWrite.
> 
> 
> Levente
> 
> On Thu, 17 Oct 2013, Levente Uzonyi wrote:
> 
> >
> >Seems like the end of the message got lost. So here's the full message 
> >again:
> >
> >Hello,
> >
> >we've been experiencing memory leakage in long running Squeak images using 
> >SqueakSSL. After a bit of monitoring I found that 132 bytes get leaked
> >for each https request done from the image. After a bit of code review, 
> >I've probably found the culprit, and another potential source of memory
> >leak. For the reference, the source file this mail is about is
> >http://squeakvm.org/cgi-bin/viewvc.cgi/squeak/branches/Cog/platforms/unix/plugins/SqueakSSL/sqUnixOpenSSL.c?revision=2713&view=markup
> >
> >The main memory leak is in sqDestroySSL function (starting on line 117), 
> >which doesn't free the bioRead and bioWrite variables (allocated by
> >sqCreateSSL on line 98-99) of the ssl object. My suggested solution is to 
> >insert the following two lines before line 132:
> >
> >       BIO_free_all(ssl->bioRead);
> >       BIO_free_all(ssl->bioWrite);
> >
> >The other potential source of memory leak is sqSetStringPropertySSL 
> >(starting on line 381). It allocates a chunk of memory on line 389, but
> >doesn't use nor free it, if the propID argument is not 
> >SQSSL_PROP_CERTNAME. My suggested solution is to insert the following line 
> >after line 396:
> >
> >       if(property) free(property);
> >
> >Note that I haven't tested any of these, but I hope someone who is more 
> >into VM building right now will try them.
> >
> >Cheers,
> >Levente
> >