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

Hi,

On 2021-03-16 12:31:17 -0400, Tom Lane wrote:
> However, how sure are you really that this specific bug accounts for
> all of the leakage you saw?

I'd not be surprised if there were more...

> I'm wondering about that because I see some other stuff in be_tls_init()
> that looks like it might get leaked, notably the root_cert_list read
> from the ssl_ca_file.

I think that specific instance should be, at least in the non-error
paths, fine though:

https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_client_CA_list.html
> SSL_CTX_set_client_CA_list() sets the list of CAs sent to the client
> when requesting a client certificate for ctx. Ownership of list is
> transferred to ctx and it should not be freed by the caller.

So it seems like we'd need cleanup root_cert_list in case of errors
(we'd not reach the SSL_CTX_set_client_CA_list), but not otherwise?

Given that we're careful to destroy the "temporary" ssl context in case
of error, perhaps the best way to deal with root_cert_list being freed
in case of error would be to assign it to the context as soon as its
loaded?

> This code was originally meant to be run exactly
> once at postmaster start, so it's not too surprising that it's not as
> careful as it now needs to be.

Yea.

Greetings,

Andres Freund