Discussion:
Leak in BN_rand_range?
Jeffrey Walton
2014-09-24 16:27:29 UTC
Permalink
I've got a program that repeatedly calls BN_rand_range. Valgrind is
reporting 2.4 MB of leaks.

If I comment out the loop that generates the range value, then the
leak summary drops to 0.

Is there anything else I should be doing below?

**********

Error checking was removed from the sample, but nothing fails.

#include <assert.h>
#include <stdlib.h>
#include <errno.h>

#include <openssl/bn.h>

#define ITERATIONS 1000UL

int main(int argc, char* argv[])
{
UNUSED(argc), UNUSED(argv);

int rc = 0, err;
BIGNUM *range = NULL, *value = NULL;

range = BN_new();
rc = BN_set_word(range, 3);

for(size_t i = 0; i < ITERATIONS; i++)
{
if(value) {
BN_free(value), value = NULL;
}

rc = BN_rand_range(value, range);
}

if(range) {
BN_free(range), range = NULL;
}

if(value) {
BN_free(value), value = NULL;
}

return 0;
}
______________________________________________________________________
OpenSSL Project http://www.openssl.org
User Support Mailing List openssl-users-MCmKBN63+***@public.gmane.org
Automated List Manager majordomo-MCmKBN63+***@public.gmane.org
Mounir IDRASSI
2014-09-24 17:04:04 UTC
Permalink
Hi,

The leak comes from the fact that you are passing a NULL "value"
parameter to BN_rand_range. This is unexpected as this is where the
result is supposed to be written. Internally, because of this NULL
pointer, OpenSSL allocate temporary BIGNUM that gets lost (allocated in
the call to BN_bin2bn inside the function bnrand at line 199 of bn_rand.c).

To avoid this leak, just allocate your "value" variable at the begining
and don't free it inside the loop because its value will be updated by
BN_rand_range. So just add value = BN_new(); at the begining and remove
the if block inside the loop.

Cheers,
--
Mounir IDRASSI
IDRIX
http://www.idrix.fr
Post by Jeffrey Walton
I've got a program that repeatedly calls BN_rand_range. Valgrind is
reporting 2.4 MB of leaks.
If I comment out the loop that generates the range value, then the
leak summary drops to 0.
Is there anything else I should be doing below?
**********
Error checking was removed from the sample, but nothing fails.
#include <assert.h>
#include <stdlib.h>
#include <errno.h>
#include <openssl/bn.h>
#define ITERATIONS 1000UL
int main(int argc, char* argv[])
{
UNUSED(argc), UNUSED(argv);
int rc = 0, err;
BIGNUM *range = NULL, *value = NULL;
range = BN_new();
rc = BN_set_word(range, 3);
for(size_t i = 0; i < ITERATIONS; i++)
{
if(value) {
BN_free(value), value = NULL;
}
rc = BN_rand_range(value, range);
}
if(range) {
BN_free(range), range = NULL;
}
if(value) {
BN_free(value), value = NULL;
}
return 0;
}
______________________________________________________________________
OpenSSL Project http://www.openssl.org
______________________________________________________________________
OpenSSL Project http://www.openssl.org
User Support Mailing List openssl-users-MCmKBN63+***@public.gmane.org
Automated List Manager majordomo-MCmKBN63+***@public.gmane.org
Jeffrey Walton
2014-09-24 17:23:27 UTC
Permalink
On Wed, Sep 24, 2014 at 1:04 PM, Mounir IDRASSI
Post by Mounir IDRASSI
The leak comes from the fact that you are passing a NULL "value"
parameter to BN_rand_range. This is unexpected as this is where the
result is supposed to be written. Internally, because of this NULL
pointer, OpenSSL allocate temporary BIGNUM that gets lost (allocated in
the call to BN_bin2bn inside the function bnrand at line 199 of bn_rand.c).
To avoid this leak, just allocate your "value" variable at the begining
and don't free it inside the loop because its value will be updated by
BN_rand_range. So just add value = BN_new(); at the begining and remove
the if block inside the loop.
Oh, that's interesting. I incorrectly tested for 0 as success (and not
1). And the program did not segfault...

Thanks for the help.
______________________________________________________________________
OpenSSL Project http://www.openssl.org
User Support Mailing List openssl-users-MCmKBN63+***@public.gmane.org
Automated List Manager majordomo-MCmKBN63+***@public.gmane.org
Loading...