pg_dump: fix memory leak
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
Attachments:
0001-pg_dump-fix-memory-allocation.patchtext/x-diff; name=0001-pg_dump-fix-memory-allocation.patchDownload
From d776c7c5f60d3798fe529ca2f5f84e878ef6e424 Mon Sep 17 00:00:00 2001
From: Maksim Korotkov <m.korotkov@postgrespro.ru>
Date: Thu, 28 Aug 2025 16:55:04 +0300
Subject: [PATCH] pg_dump: fix memory allocation
If the function pg_strong_random() return false, memory allocated to the ret pointer
is leaked.
Fixes: 71ea0d67954 ("Restrict psql meta-commands in plain-text dumps.")
Found by PostgresPro with Svace Static Analyzer.
Signed-off-by: Maksim Korotkov <m.korotkov@postgrespro.ru>
---
src/bin/pg_dump/dumputils.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 8945bdd42c5..65ac9084d91 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -968,11 +968,12 @@ char *
generate_restrict_key(void)
{
uint8 buf[64];
- char *ret = palloc(sizeof(buf));
+ char *ret = NULL;
if (!pg_strong_random(buf, sizeof(buf)))
return NULL;
+ ret = palloc(sizeof(buf));
for (int i = 0; i < sizeof(buf) - 1; i++)
{
uint8 idx = buf[i] % strlen(restrict_chars);
--
2.34.1
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