From: "h.shirosaki (Hiroshi Shirosaki)" <h.shirosaki@...>
Date: 2013-04-29T01:11:38+09:00
Subject: [ruby-core:54670] [ruby-trunk - Bug #8337][Closed] Test failure and memory leak with OpenSSL::BN


Issue #8337 has been updated by h.shirosaki (Hiroshi Shirosaki).

Status changed from Assigned to Closed

nagachika (Tomoyuki Chikanaga) wrote:
> Hello,
> 
> I think ALLOCA_N() uses alloca() to allocate memory from machine stack and xfree() is not necessary.

I use ALLOCA_N only for Fixnum and ALLOC_N is used for Bignum that needs xfree().

https://github.com/ruby/ruby/blob/be4aa330374d42cdead52a94144be189b5054e67/ext/openssl/ossl_bn.c#L145
----------------------------------------
Bug #8337: Test failure and memory leak with OpenSSL::BN
https://bugs.ruby-lang.org/issues/8337#change-39011

Author: h.shirosaki (Hiroshi Shirosaki)
Status: Closed
Priority: Normal
Assignee: h.shirosaki (Hiroshi Shirosaki)
Category: ext
Target version: current: 2.1.0
ruby -v: ruby 2.1.0dev (2013-04-27 trunk 40468) [x86_64-darwin12.3.0]
Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN


I noticed test failure of test_to_bn.

http://ci.rubyinstaller.org/job/ruby-trunk-x64-test-all/1137/console
  1) Failure:
test_to_bn(OpenSSL::TestBN) [C:/Users/Worker/Jenkins/workspace/ruby-trunk-x64-build/test/openssl/test_bn.rb:36]:
<#<OpenSSL::BN:0x00000017d20188>> expected but was
<#<OpenSSL::BN:0x00000017d18460>>.

sizeof(VALUE) of the following code looks wrong because sizeof(VALUE) > sizeof(long) on x64 Windows.

https://github.com/ruby/ruby/blob/8b29525dadeaba1ba6dc2a9ea5e590aa9d1d825a/ext/openssl/ossl_bn.c#L130

And ALLOC_N() may need xfree().
I've confirmed memory leak with the following script.

$ ruby -rOpenSSL -e 'loop { 1.to_bn }'


Here is a patch to fix above issues.

diff --git a/ext/openssl/ossl_bn.c b/ext/openssl/ossl_bn.c
index 4e9734e..3d8e095 100644
--- a/ext/openssl/ossl_bn.c
+++ b/ext/openssl/ossl_bn.c
@@ -127,15 +127,17 @@ ossl_bn_initialize(int argc, VALUE *argv, VALUE self)
 	long n = FIX2LONG(str);
 	unsigned long un = labs(n);
 
-	for (i = sizeof(VALUE) - 1; 0 <= i; i--) {
+	for (i = sizeof(long) - 1; 0 <= i; i--) {
 	    bin[i] = un&0xff;
 	    un >>= 8;
 	}
 
 	GetBN(self, bn);
 	if (!BN_bin2bn(bin, sizeof(long), bn)) {
+	    xfree(bin);
 	    ossl_raise(eBNError, NULL);
 	}
+	xfree(bin);
 	if (n < 0) BN_set_negative(bn, 1);
 	return self;
     }
@@ -154,8 +156,10 @@ ossl_bn_initialize(int argc, VALUE *argv, VALUE self)
 
 	GetBN(self, bn);
 	if (!BN_bin2bn(bin, (int)sizeof(BDIGIT)*RBIGNUM_LENINT(str), bn)) {
+	    xfree(bin);
 	    ossl_raise(eBNError, NULL);
 	}
+	xfree(bin);
 	if (!RBIGNUM_SIGN(str)) BN_set_negative(bn, 1);
 	return self;
     }



-- 
http://bugs.ruby-lang.org/