public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jacob Champion <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Cc: Daniel Schreiber <[email protected]>
Subject: Re: PostgreSQL 17: Bug in libpq when libpq is dlopened/closed multiple times
Date: Wed, 22 Apr 2026 11:29:04 -0700
Message-ID: <CAOYmi+kac3wEE3iqxHfHCNd_n2i-Or=n+Qk8_G24UZn2uz3DyQ@mail.gmail.com> (raw)
In-Reply-To: <CAOYmi+=e9bQQa1jgLqZ18m5w-7KA44VeL9FON6i2gd8bi=d5Jw@mail.gmail.com>
References: <[email protected]>
<CAOYmi+=e9bQQa1jgLqZ18m5w-7KA44VeL9FON6i2gd8bi=d5Jw@mail.gmail.com>
[moving to -hackers]
On Fri, Apr 17, 2026 at 12:14 PM Jacob Champion
<[email protected]> wrote:
>
> On Fri, Apr 17, 2026 at 7:33 AM Daniel Schreiber
> <[email protected]> wrote:
> > my colleagues and I probably found a bug in libpq when libpq is dlopened
> > and closed multiple times during the lifetime of a process. In our setup
> > we use a PAM module which links to libpq. The process using PAM is
> > linked against openssl, so openssl is loaded during the complete
> > lifetime of the process whereas libpq is loaded only during PAM
> > authentication (and unloaded when PAM has finished).
> >
> > [snip]
> >
> > According to our findings every time a connection is established after
> > dlopening libpq one of the 127 available BIO_METHOD structures in
> > OpenSSL is consumed:
> > https://github.com/postgres/postgres/blob/REL_17_9/src/interfaces/libpq/fe-secure-openssl.c#L1987
>
> Right. I think in this *particular* case, we should simply skip the
> call to BIO_get_new_index(). We don't need it, IIUC.
Attached is a proposal to do that.
> But I think we may also need to set expectations on whether or not
> infinite dlopen/dlclose loops are supported in general. If we ever
> come across a situation in which a call to BIO_get_new_index() is
> necessary, that leak just fundamentally can't be plugged. The same is
> true for any third-party libraries (or their dependencies, or
> theirs...) that require "one-time", irreversible calls which can't be
> tracked after we're unloaded. And we can't push these concerns up to
> the top level application developer, because they don't know we exist.
>
> (I'd be surprised if this were the only such resource leak across all
> supported versions and combinations of Kerberos, OpenSSL, OpenLDAP,
> Curl, etc. etc. From a quick search, you're the first to report this
> in the ten years since the leak was introduced, so there may be more
> dragons where you're headed.)
If anyone has thoughts on that, I'd love to hear them. I don't mind
removing this unnecessary code in HEAD, or even backpatching as a
courtesy -- but if it were up to me, I would not guarantee zero global
resource leaks across libpq and its entire dependency graph. (Even if
we magically had control over all those dependencies, I think it'd
still be reasonable for libpq devs to use "allocate once and move on"
patterns... and I want to continue using those in my new code.)
Thanks,
--Jacob
Attachments:
[application/octet-stream] 0001-Remove-call-to-BIO_get_new_index-in-OpenSSL-code.patch (3.0K, 2-0001-Remove-call-to-BIO_get_new_index-in-OpenSSL-code.patch)
download | inline diff:
From 800678db5674b0321f63fb420f942fb543b8d722 Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Mon, 20 Apr 2026 15:29:54 -0700
Subject: [PATCH] Remove call to BIO_get_new_index() in OpenSSL code
BIO_meth_new() takes an "index type" as its first argument. Older
OpenSSL documentation used to suggest that this argument should be
constructed by registering a custom index with BIO_get_new_index() and
combining that with the appropriate "BIO class" bit.
However, custom BIO indices are an extremely limited resource [1], and
newer documentation suggests that clients should only take one if they
expect to search a BIO chain for it later:
`type` can be set to either `BIO_TYPE_NONE` or via BIO_get_new_index()
if a unique type is required for searching[...] Note that
BIO_get_new_index() can only be used 127 times before it returns an
error.
We don't fall into that category (we immediately discard the index we've
created), and it doesn't look like OpenSSL has ever required a nonzero
index, so avoid registering one altogether.
Per complaint by Daniel Schreiber that libpq eventually breaks OpenSSL
when repeatedly dlopen/dlclose'd. It's not clear to me that we support
that use case in general (related TODO: decide whether to backpatch
this), but this change seems like a clear improvement going forward.
[1] https://github.com/openssl/openssl/issues/23655
Reported-by: Daniel Schreiber <[email protected]>
Discussion: https://postgr.es/m/f7fe39b3-7e99-4939-8852-07350549161d%40hrz.tu-chemnitz.de
Backpatch-through: TODO
---
src/backend/libpq/be-secure-openssl.c | 9 ++-------
src/interfaces/libpq/fe-secure-openssl.c | 8 +-------
2 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index a3e222f3a3d..6c3717bc024 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -1419,13 +1419,8 @@ port_bio_method(void)
{
if (!port_bio_method_ptr)
{
- int my_bio_index;
-
- my_bio_index = BIO_get_new_index();
- if (my_bio_index == -1)
- return NULL;
- my_bio_index |= BIO_TYPE_SOURCE_SINK;
- port_bio_method_ptr = BIO_meth_new(my_bio_index, "PostgreSQL backend socket");
+ port_bio_method_ptr = BIO_meth_new(BIO_TYPE_SOURCE_SINK,
+ "PostgreSQL backend socket");
if (!port_bio_method_ptr)
return NULL;
if (!BIO_meth_set_write(port_bio_method_ptr, port_bio_write) ||
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index fbd3c63fb5d..2214a141847 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1841,13 +1841,7 @@ pgconn_bio_method(void)
if (!pgconn_bio_method_ptr)
{
- int my_bio_index;
-
- my_bio_index = BIO_get_new_index();
- if (my_bio_index == -1)
- goto err;
- my_bio_index |= BIO_TYPE_SOURCE_SINK;
- res = BIO_meth_new(my_bio_index, "libpq socket");
+ res = BIO_meth_new(BIO_TYPE_SOURCE_SINK, "libpq socket");
if (!res)
goto err;
--
2.34.1
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: PostgreSQL 17: Bug in libpq when libpq is dlopened/closed multiple times
In-Reply-To: <CAOYmi+kac3wEE3iqxHfHCNd_n2i-Or=n+Qk8_G24UZn2uz3DyQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox