On 08/26/2016 11:31 AM, Heikki Linnakangas wrote:
> On 07/05/2016 04:46 PM, Andreas Karlsson wrote:
>> @@ -280,8 +287,9 @@ px_find_digest(const char *name, PX_MD **res)
>> digest = px_alloc(sizeof(*digest));
>> digest->algo = md;
>>
>> - EVP_MD_CTX_init(&digest->ctx);
>> - if (EVP_DigestInit_ex(&digest->ctx, digest->algo, NULL) == 0)
>> + digest->ctx = EVP_MD_CTX_create();
>> + EVP_MD_CTX_init(digest->ctx);
>> + if (EVP_DigestInit_ex(digest->ctx, digest->algo, NULL) == 0)
>> return -1;
>>
>> h = px_alloc(sizeof(*h));
>
> Now that we're calling EVP_MD_CTX_create((), which allocates memory, are
> we risking memory leaks? It has always been part of the contract that
> you have to call px_md_free(), for any context returned by
> px_find_digest(), but I wonder just how careful we have been about that.
> Before this, you would probably get away with it without leaking, if the
> digest implementation didn't allocate any extra memory or other resources.
>
> At least pg_digest and try_unix_std functions call px_find_digest(), and
> then do more palloc()s which could elog() if you run out of memory,
> leaking th digest struct. Highly unlikely, but I think it would be
> fairly straightforward to reorder those calls to eliminate the risk, so
> we probably should.
Since px_find_digest() calls palloc() later in the function there is a
slim possibility of memory leaks. How do we generally handle that things
not allocated with palloc() may leak when something calls elog()?
I have attached new versions of the patches which are rebased on master,
with slightly improves error handling in px_find_digest(), and handles
the deprecation of ASN1_STRING_data().
Andreas
