[omniORB] Patch to fix leak in SSL context initialization

[Peter Klotz]

Hello

Initializing and destroying an SSL enabled omniORB in a loop results in
memory leaks because the SSL context string parameters are allocated but
never freed.

These 3 lines in sslTransportImpl.cc cause the problem:

sslContext::certificate_authority_file = CORBA::string_dup(value);
sslContext::key_file = CORBA::string_dup(value);
sslContext::key_file_password = CORBA::string_dup(value);

The attached patch against omniORB 4.1.4 simply calls
CORBA::string_free() before allocating the new value.

Since all values are zero initialized on startup the fix should be safe.

Regards, Peter.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: omniORB-4.1.4-sslContextInitialization.patch
Type: text/x-patch
Size: 1053 bytes
Desc: not available
Url : http://www.omniorb-support.com/pipermail/omniorb-list/attachments/20091007/e0edf69c/omniORB-4.1.4-sslContextInitialization.bin

[Peter Klotz]

Post by Peter Klotz
Hello
Initializing and destroying an SSL enabled omniORB in a loop results in
memory leaks because the SSL context string parameters are allocated but
never freed.
sslContext::certificate_authority_file = CORBA::string_dup(value);
sslContext::key_file = CORBA::string_dup(value);
sslContext::key_file_password = CORBA::string_dup(value);
The attached patch against omniORB 4.1.4 simply calls
CORBA::string_free() before allocating the new value.
Since all values are zero initialized on startup the fix should be safe.

Hello Duncan

Could you please apply this patch or do you have any objections?

Regards, Peter.

[Duncan Grisby]

Post by Peter Klotz
Could you please apply this patch or do you have any objections?

Sorry for not replying sooner.

Unfortunately, I can't apply your fix. It's documented that it is valid
to assign values directly to the sslContext static variables (it's what
the ssl_echo example program does too). Therefore, the pointers in
sslContext::key_file etc., are not necessarily things that can be freed
with CORBA::string_free.

Without changing the API (and thus breaking binary compatibility with
the library), it's not possible to simply free the strings. The only
option would be to maintain a flag for each string, indicating whether
it should be freed or not. In that case, the freeing should happen in
the initialiser detach() method.

Do you have real code that repeatedly destroys and reinitialises the
ORB, or is it just a test?

Cheers,

Duncan.

[Peter Klotz]

Post by Duncan Grisby
Unfortunately, I can't apply your fix. It's documented that it is valid
to assign values directly to the sslContext static variables (it's what
the ssl_echo example program does too). Therefore, the pointers in
sslContext::key_file etc., are not necessarily things that can be freed
with CORBA::string_free.
Without changing the API (and thus breaking binary compatibility with
the library), it's not possible to simply free the strings. The only
option would be to maintain a flag for each string, indicating whether
it should be freed or not. In that case, the freeing should happen in
the initialiser detach() method.

I see.

Wouldn't it make sense to change that behavior for 4.2.x (or whatever is
next) and to make these static variables private?

Is it really necessary to have two quite different ways to set SSL
initialization parameters (setting them directly vs. using ORB_init())?

Post by Duncan Grisby
Do you have real code that repeatedly destroys and reinitialises the
ORB, or is it just a test?

It is production code but I can live with my patch for the time being.

Regards, Peter.