[PATCH] Fix memory leak in pg_config

Sorry for the formatting in my previous email. Resending in plain text.

Hi,

While testing PostgreSQL with LeakSanitizer enabled, I found a memory
leak in pg_config.

The issue can be reproduced by running:

pg_config --version

LeakSanitizer reports 2829 bytes leaked in 47 allocations. The full
report is attached as leak_sanitizer.txt.

The relevant allocation stack is:

libc.so.6!__GI___libc_malloc(size_t bytes) (malloc.c:3288)
pg_malloc_internal(size_t size, int flags)
(src/common/fe_memutils.c:36)
palloc(Size size)
(src/common/fe_memutils.c:123)
palloc_mul(Size s1, Size s2)
(src/common/fe_memutils.c:312)
get_configdata(const char *my_exec_path, size_t *configdata_len)
(src/common/config_info.c:42)
main(int argc, char **argv)
(src/bin/pg_config/pg_config.c:157)

Although get_configdata() uses palloc_array() and pstrdup(), pg_config
is a frontend program. The frontend implementations of palloc() and
pstrdup() ultimately allocate memory using malloc() and strdup(),
through pg_malloc_internal() and pg_strdup(), respectively. These
allocations are not owned by a backend memory context and therefore
remain allocated until they are explicitly freed or the process exits.

get_configdata() allocates the ConfigData array with palloc_array() and
allocates its name and setting strings with pstrdup(). pg_config does
not release these allocations before exiting.

This leak is harmless for a short-lived process, since the operating
system releases the memory at process termination. However, it causes
LeakSanitizer failures and adds noise to sanitizer-based testing.

The attached patch adds a helper that frees every name and setting
string and then frees the ConfigData array. The helper is called on
every exit path reached after get_configdata().

After applying the patch, pg_config --version completes without a
LeakSanitizer report.

The patch is attached as:

v1-0001-Fix-memory-leak-in-pg_config.patch

--
Best wishes,
Ivan Kush
Tantor Labs LLC

Hi Ivan,

LeakSanitizer reports 2829 bytes leaked in 47 allocations.

I can confirm the leak and the fix. The 47 allocations are exactly 23
names + 23 settings + 1 array. With your patch the tool reports zero.

Fun archaeology: this leak celebrates its 10th anniversary this year.
It arrived with a5c43b88694 (PG 9.6), which introduced get_configdata()
with the comment "the caller is responsible for pfreeing the result".
pg_config has been ignoring that contract ever since. Before 9.6 it
used static buffers and had nothing to leak at all. Simpler times.

One thing to be aware of: our CI runs sanitizers with
detect_leaks=0, and the comment there says "too many uninteresting
leak errors in short-lived binaries". This leak is a prime specimen
of the genre, so the answer here may well be "the OS frees it faster
than we do". If the goal is quiet LSan runs, a suppression list might
scale better: there are many binaries other than pg_config that leak
too. Anyway, I think it would be good to quite LSan if possible...

Two small notes:

- Validating the arguments before calling get_configdata() would
leave a single free before return instead of three call sites.
- The function definition puts "static" alone on its own line. The
usual style is "static void" together, then the function name on
the next line.

Thank you!

Best regards, Andrey Borodin.

Thanks for the review.

LeakSanitizer is widely used tools, so I think it
would be useful to make PostgreSQL work cleanly with them. We could
either fix reported leaks where cleanup is straightforward, or maintain
suppressions for intentional process-lifetime allocations. I prefer
fixing the leaks where possible.

We use these sanitizers in our own work, so we can continue examining
their reports across PostgreSQL, fix actual leaks, add suppressions where
appropriate, and submit the changes as separate patches.

For pg_config, the cleanup is small and straightforward. A broad
suppression for get_configdata() could also hide future leaks from the
same call path.

In v2, I restructured the control flow using if/else, so
free_configdata() is called only once before returning from main().
Invalid arguments now set the exit code and break out of the loop
instead of calling exit().

Personally, I prefer the simpler control flow used in v1.

I also corrected the function definition style.

The updated patch is attached.

On 26-07-21 21:35, Andrey Borodin wrote:

Hi Ivan,

LeakSanitizer reports 2829 bytes leaked in 47 allocations.

I can confirm the leak and the fix. The 47 allocations are exactly 23
names + 23 settings + 1 array. With your patch the tool reports zero.

Fun archaeology: this leak celebrates its 10th anniversary this year.
It arrived with a5c43b88694 (PG 9.6), which introduced get_configdata()
with the comment "the caller is responsible for pfreeing the result".
pg_config has been ignoring that contract ever since. Before 9.6 it
used static buffers and had nothing to leak at all. Simpler times.

One thing to be aware of: our CI runs sanitizers with
detect_leaks=0, and the comment there says "too many uninteresting
leak errors in short-lived binaries". This leak is a prime specimen
of the genre, so the answer here may well be "the OS frees it faster
than we do". If the goal is quiet LSan runs, a suppression list might
scale better: there are many binaries other than pg_config that leak
too. Anyway, I think it would be good to quite LSan if possible...

Two small notes:

- Validating the arguments before calling get_configdata() would
leave a single free before return instead of three call sites.
- The function definition puts "static" alone on its own line. The
usual style is "static void" together, then the function name on
the next line.

Thank you!

Best regards, Andrey Borodin.

--
Best wishes,
Ivan Kush
Tantor Labs LLC

Hi,

On 2026-07-21 23:35:18 +0500, Andrey Borodin wrote:

One thing to be aware of: our CI runs sanitizers with
detect_leaks=0, and the comment there says "too many uninteresting
leak errors in short-lived binaries".

So far our policy is that we do not care about leaks in short lived
binaries. I doubt that we need more very verbose projects at this point, so I
don't think we'll want to change this in the near future.

Greetings,

Andres Freund

On Tue, Jul 21, 2026 at 05:05:14PM -0400, Andres Freund wrote:

So far our policy is that we do not care about leaks in short lived
binaries. I doubt that we need more very verbose projects at this point, so I
don't think we'll want to change this in the near future.

None of the changes proposed in the patch are worth caring about. The
only memory leaks I would care about in the frontend binaries are
those where we could run a tool in an infinite loop where the
non-freed would bloat. One example of that is pg_receivewal without
--no-loop, that could be used as a service to get WAL from a remote
source.
--
Michael

If the project policy is to ignore leaks in short-lived binaries, why
are those known cases not listed explicitly in an LSan suppression file
(for example, see attached patch)?

Disabling leak detection globally with detect_leaks=0 also hides leaks
that may be relevant in long-running processes. Explicit suppressions
would document the intentionally ignored cases while preserving LSan
coverage for the rest of the code.

Regards,
Ivan

On 26-07-22 01:38, Michael Paquier wrote:

None of the changes proposed in the patch are worth caring about. The
only memory leaks I would care about in the frontend binaries are
those where we could run a tool in an infinite loop where the
non-freed would bloat. One example of that is pg_receivewal without
--no-loop, that could be used as a service to get WAL from a remote
source.
--
Michael

--
Best wishes,
Ivan Kush
Tantor Labs LLC

Hi,

On 2026-07-22 14:54:17 +0300, Ivan Kush wrote:

If the project policy is to ignore leaks in short-lived binaries, why
are those known cases not listed explicitly in an LSan suppression file (for
example, see attached patch)?

Disabling leak detection globally with detect_leaks=0 also hides leaks
that may be relevant in long-running processes. Explicit suppressions
would document the intentionally ignored cases while preserving LSan
coverage for the rest of the code.

Because we so far have not been using lsan. It might be worth supporting it,
but we haven't so far. You'd have to make a case for doing that on its own.

For the server lsan is probably not very helpful. Just about all allocations
in the backend are done via memory contexts, which lsan does not understand -
and can't easily be made to be understand, IIRC. Whereas we made valgrind
mostly understand memory contexts.

Greetings,

Andres Freund