Re: BUG #16160: Minor memory leak in case of starting postgres server with SSL encryption

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: duspensky(at)ya(dot)ru, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #16160: Minor memory leak in case of starting postgres server with SSL encryption
Date: 2019-12-13 06:39:15
Message-ID: 20191213063915.GE1942@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Wed, Dec 11, 2019 at 04:22:03PM +0000, PG Bug reporting form wrote:
> According to the following information
>
> https://wiki.openssl.org/index.php/Diffie-Hellman_parameters
>
> DH_free function must be called after SSL_CTX_set_tmp_dh

That's not directly mentioned on their docs actually:
https://www.openssl.org/docs/man1.0.2/man3/SSL_CTX_set_tmp_dh.html

But it seems to me that you are right. If I look at the OpenSSL code,
ssl3_ctrl() does a DH_free() on error when going though
SSL_CTRL_SET_TMP_DH, and the code copies the DH directly using
DHparams_dup() so we don't need to keep any reference to it in our
code.

One more disturbing issue is that we can would accumulate garbage if
we keep reloading the SSL context in the postmaster. For this reason,
it could justify a backpatch down to the point where SSL parameters
are reloadable. On the other hand, the leak is small, so my take
is actually to just fix HEAD and call it a day.

Attached is a patch, I'll go commit that if there are no objections.
The DH handling does not really change regarding the way it gets
free'd or not down to 0.9.8.
--
Michael

Attachment Content-Type Size
bessl-dh-free.patch text/x-diff 391 bytes

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Manuel Rigger 2019-12-13 09:56:09 Re: FailedAssertion("!OidIsValid(def->collOid)", File: "view.c", Line: 89)
Previous Message Michael Paquier 2019-12-13 03:45:36 Re: REINDEX CONCURRENTLY unexpectedly fails