Re: using palloc/pfree for OpenSSL allocations with CRYPTO_set_mem_functions

Thank you for the explanation! I looked into what OpenSSL does when freeing
the underlying OSSLCipher resources, and as you say, it is more than just a
free. There is sometimes aditional values to free, and they go through and
overwrite the data in a special way regardless. So I see why the
TopMemoryContext approach is needed either way.

My particular interest was in using the BIGNUM implementation from OpenSSL
within my project, but the BIGNUM frees also do more than just freeing. So
it seems there is no way around the TopMemoryContext in my case either.

I could imagine that there may be some effect on the particular character
of memory fragmentation that comes with their malloc/free vs palloc/pfree,
but that's definitely outside of my personal purposes and outside of my
expertise to evaluate even at a high level!

So thank you again for helping me understand this! Happy to have a much
clear understanding of the TopMemoryContext approach!
Evan

On Thu, Feb 1, 2024 at 5:01 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Evan Czaplicki <evancz(at)gmail(dot)com> writes:
> > I noticed that OpenSSL has a CRYPTO_set_mem_functions
> > <https://www.openssl.org/docs/man3.2/man3/CRYPTO_set_mem_functions.html>
> > function:
>
> >> If no allocations have been done, it is possible to “swap out” the
> default
> >> implementations for OPENSSL_malloc(), OPENSSL_realloc() and
> OPENSSL_free()
> >> and replace them with alternate versions.
>
> > But a different technique is used in contrib/pgcrypto/openssl.c
>
> >> To make sure we don't leak OpenSSL handles on abort, we keep OSSLCipher
> >> objects in a linked list, allocated in TopMemoryContext. We use the
> >> ResourceOwner mechanism to free them on abort.
>
> > Would it be desirable to do this? If not, why is the TopMemoryContext
> > approach a better option? I do not understand the code quite well enough
> to
> > evaluate the tradeoffs myself yet!
>
> Seems to me that these address different purposes. If we put in a
> CRYPTO_set_mem_functions layer, I doubt that we'd have any good idea
> of which allocations are used for what. So we could not replace what
> pgcrypto is doing with a simple MemoryContextReset (even if we cared
> to assume that freeing an OSSLCipher involves only free() operations
> and no other resources). I think the only real win we'd get from
> such a layer is that OpenSSL's allocations would be better exposed
> for accounting purposes, eg the pg_backend_memory_contexts view.
> That's not negligible, but I don't find it a compelling reason to
> do the work, either.
>
> regards, tom lane
>