| From: | Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | Matt Carter <Matt(dot)Carter(at)twosigma(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, "pgsql-bugs(at)lists(dot)postgresql(dot)org" <pgsql-bugs(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: BUG #19411: libpq 16.x exhibits a memory leak when connections are repeatedly created and destroyed |
| Date: | 2026-10-30 19:00:07 |
| Message-ID: | CAOYmi+=fdEq0VMx0Rqn=5C06wLdZ-h_5_xR2eWHFGv1O_Hg0QQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
On Fri, Oct 30, 2026 at 9:55 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Thanks for testing that, but after looking closer, I believe that
> pg_GSS_continue has even more problems here. It will attempt to
> acquire credentials whether or not GSS delegation is enabled,
> contrary to the adjacent comment and unlike what happens in
> pqsecure_open_gss. And if GSS delegation is enabled, it will
> set the GSS_C_DELEG_FLAG whether or not it was able to acquire
> credentials, which seems not very sane and again different from
> the logic in pqsecure_open_gss.
Hi! I don't have any more time today to dig, but I took a partial look.
+1 for the name change to pg_GSS_get_cred_cache().
> /* Only try to acquire credentials if GSS delegation isn't disabled. */
> - if (!pg_GSS_have_cred_cache(&conn->gcred))
> - conn->gcred = GSS_C_NO_CREDENTIAL;
> -
> if (conn->gssdelegation && conn->gssdelegation[0] == '1')
> - gss_flags |= GSS_C_DELEG_FLAG;
> + {
> + if (conn->gcred == GSS_C_NO_CREDENTIAL)
> + conn->gcred = pg_GSS_get_cred_cache();
> + if (conn->gcred != GSS_C_NO_CREDENTIAL)
> + gss_flags |= GSS_C_DELEG_FLAG;
> + }
This might be equivalent logic, but I'm not sure. The current code
always requests delegation if conn->gssdelegation is true (so if
pg_GSS_get_cred_cache()/gss_acquire_cred() fail, then delegation will
still be attempted with GSS_C_NO_CREDENTIAL). The new code requests
delegation only if gss_acquire_cred() succeeds -- if that fails but
the later GSS_C_NO_CREDENTIAL attempt succeeds for some weird
GSS-specific reason, shouldn't we still try to delegate as directed?
(Maybe if gss_acquire_cred() fails, the later GSS_C_NO_CREDENTIAL
attempt is also guaranteed to fail? But if that's true then this code
should look a lot different IMO.)
Separately from that:
> + if (conn->gcred == GSS_C_NO_CREDENTIAL)
> + conn->gcred = pg_GSS_get_cred_cache();
Are there any delegation cases where we were actually expecting to
overwrite the previous, valid, credentials? I deleted
pg_GSS_continue's pg_GSS_have_cred_cache() call entirely, and the
tests still passed on my machine, which makes me worried that maybe
there's an edge case that was never recorded. (Alternatively, maybe
the call is completely unnecessary.)
Thanks,
--Jacob
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Rachitskiy | 2026-10-30 19:10:10 | Re: BUG #19815: 004_sni.pl fails deterministically with OpenSSL 1.1.1 (tests 75 and 86) |
| Previous Message | Tom Lane | 2026-10-30 17:36:56 | Re: BUG #19411: libpq 16.x exhibits a memory leak when connections are repeatedly created and destroyed |