[openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-11 Thread Emilia Käsper via RT
Yep, there is no need to clean up early here (we don't guarantee that errored calls leave everything in a pristine unmodified state). Plus this does indeed forget to zero the pointer. Closing. Thanks for submitting, though, and thanks David for the review! -- Ticket here:

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-10 Thread David Benjamin via RT
By the way, returning the original subject, I don't believe there is a leak here. If EC_GROUP_copy fails, dest still exists and is owned by the caller. It's the caller's obligation to call EC_GROUP_free and that will release the partially-copied EC_GROUP. (Which will, with this patch, cause a

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-09 Thread Bill Parker via RT
Geez, What did I start here (egad) :) Bill On Wed, Mar 9, 2016 at 5:03 AM, Salz, Rich via RT wrote: > > No, you got that right, NULL being 'safe' to free varies with OS. > > Except we mandate ANSI C which means it's portable :) > > -- > Ticket here:

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-09 Thread Salz, Rich via RT
> No, you got that right, NULL being 'safe' to free varies with OS. Except we mandate ANSI C which means it's portable :) -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4401 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe:

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-09 Thread Richard Moore
On 9 March 2016 at 05:30, Peter Waltenberg wrote: > No, you got that right, NULL being 'safe' to free varies with OS. > > ​It shouldn't if you're programming in C, from the standard (C89): The free function causes the space pointed to by ptr to be deallocated, that is, made

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-09 Thread Richard Moore via RT
On 9 March 2016 at 05:30, Peter Waltenberg wrote: > No, you got that right, NULL being 'safe' to free varies with OS. > > ​It shouldn't if you're programming in C, from the standard (C89): The free function causes the space pointed to by ptr to be deallocated, that is, made

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-08 Thread Peter Waltenberg via RT
er via RT Sent by: "openssl-dev" Date: 03/09/2016 07:53AM Cc: openssl-dev@openssl.org Subject: Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c' I must be brain dead today, since free'ing something that is already NULL is not a p

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-08 Thread Peter Waltenberg
No, you got that right, NULL being 'safe' to free varies with OS. But - you aren't calling free() directly, THIS makes it safe. That's one of the other benefits of having objects allocated and released by internal functions rather than doing it directly.void BN_MONT_CTX_free(BN_MONT_CTX *mont){   

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-08 Thread Bill Parker via RT
I must be brain dead today, since free'ing something that is already NULL is not a problem (geez)... Heh On Tue, Mar 8, 2016 at 12:01 PM, Salz, Rich via RT wrote: > > > + if (dest->mont_data != NULL) > > + BN_MONT_CTX_free(dest->mont_data); > >

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-08 Thread Salz, Rich
> >> + if (dest->mont_data != NULL) > >> + BN_MONT_CTX_free(dest->mont_data); > > > >Free routines don't need to check for non-NULL. > > Yes, don’t *have* to. But does it hurt to check? It makes folks wonder why the check is only there sometimes. It adds to code

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-08 Thread Blumenthal, Uri - 0553 - MITLL
On 3/8/16, 15:01 , "openssl-dev on behalf of Salz, Rich via RT" wrote: > >> + if (dest->mont_data != NULL) >> + BN_MONT_CTX_free(dest->mont_data); > >Free routines don't need to check for non-NULL.

Re: [openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-08 Thread Salz, Rich via RT
> + if (dest->mont_data != NULL) > + BN_MONT_CTX_free(dest->mont_data); Free routines don't need to check for non-NULL. -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4401 Please log in as guest with password guest if prompted -- openssl-dev

[openssl-dev] [openssl.org #4401] [PATCH] plug potential memory leak(s) in OpenSSL 1.1 pre 4 in 'ec_lib.c'

2016-03-08 Thread Bill Parker via RT
Hello All, In reviewing code in directory 'crypto/ec', file 'ec_lib.c'', there appears to be allocated memory which is not released when a return 0; is encountered in some cases of OPENSSL_malloc(). The patch file below should address/correct these minor leaks: --- ec_lib.c.orig