pg_dump: fix memory leak

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