pg_dump: fix memory leak
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t52186psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 27, 2026 at 08:00 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t52186_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t52186_1 && git checkout t52186_1Patchset v1 (message #1) is on t52186_1
Hi all,
I have found a potential memory leak in src/bin/pg_dump/dumputils.c in
the function generate_restrict_key().
Memory is allocated to the ret pointer if pg_strong_random returns
false, and this leads to a memory leak.
I have replaced the allocation to avoid this leak.
---
Best regards, Korotkov Maksim
PostgresPro
m.korotkov@postgrespro.ru
On 28 Aug 2025, at 16:14, m.korotkov@postgrespro.ru wrote:
Hi all,
I have found a potential memory leak in src/bin/pg_dump/dumputils.c in the function generate_restrict_key().
Memory is allocated to the ret pointer if pg_strong_random returns false, and this leads to a memory leak.
I have replaced the allocation to avoid this leak.
This is not actually a leak since the application will terminate immediately if
a restrict key cannot be generated. If you inspect the callsites you will see
this pattern:
if (!restrict_key)
restrict_key = generate_restrict_key();
if (!restrict_key)
pg_fatal("could not generate restrict key");
--
Daniel Gustafsson
<div> </div><blockquote><p><br />This is not actually a leak since the application will terminate immediately if<br />a restrict key cannot be generated.</p></blockquote><div> </div><div>Seems like this will silence static analyzer and help to find real problems in reduced warning/errors list.</div><div> </div><div>--</div><div>Roman Khapov</div>
Daniel Gustafsson wrote 2025-08-29 10:13:
This is not actually a leak since the application will terminate
immediately if
a restrict key cannot be generated.
I agree that the current usage of the function does not present a
problem, but there is no certainty that this situation will remain
unchanged. In my view, it would be prudent to explicitly release the
memory.
---
Best regards, Korotkov Maksim
PostgresPro
m.korotkov@postgrespro.ru
On 29 Aug 2025, at 09:36, m.korotkov@postgrespro.ru wrote:
Daniel Gustafsson wrote 2025-08-29 10:13:
This is not actually a leak since the application will terminate immediately if
a restrict key cannot be generated.I agree that the current usage of the function does not present a problem, but there is no certainty that this situation will remain unchanged.
I certainly hope it won't change, ignoring a failure from pg_strong_random() is
a seriously bad idea. If the function is rewritten to change its errorhandling
then allocation might be changed, right now there is no leak and no bug.
--
Daniel Gustafsson
On 2025-Aug-29, Daniel Gustafsson wrote:
On 28 Aug 2025, at 16:14, m.korotkov@postgrespro.ru wrote:
I have found a potential memory leak in src/bin/pg_dump/dumputils.c
in the function generate_restrict_key(). Memory is allocated to the
ret pointer if pg_strong_random returns false, and this leads to a
memory leak. I have replaced the allocation to avoid this leak.This is not actually a leak since the application will terminate
immediately if a restrict key cannot be generated. If you inspect the
callsites you will see this pattern:if (!restrict_key)
restrict_key = generate_restrict_key();
if (!restrict_key)
pg_fatal("could not generate restrict key");
Hmm, this begs the question -- why do we make generate_restrict_key()
return NULL in that case? Maybe we should redefine its contract to be
that it either returns a valid key, or it calls pg_fatal(). Then
callers don't have to worry about it.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
On 29 Aug 2025, at 11:52, Álvaro Herrera <alvherre@kurilemu.de> wrote:
Hmm, this begs the question -- why do we make generate_restrict_key()
return NULL in that case? Maybe we should redefine its contract to be
that it either returns a valid key, or it calls pg_fatal(). Then
callers don't have to worry about it.
That is certainly an option.
--
Daniel Gustafsson