Fix memory leak in tzparser.c
Hi hackers,
I noticed a memory leak in the addToArray() function in src/backend/utils/misc/tzparser.c.
When the override parameter is true and a duplicate timezone abbreviation is found,
the code overwrites midptr->zone without freeing the previously allocated memory.
The fix would be:
- midptr->zone = entry->zone;
+ if (midptr->zone != NULL)
+ pfree(midptr->zone);
+ midptr->zone = entry->zone;While the memory is managed by a temp memory context that gets cleaned up
eventually, the coarse-grained management might cause some memory to
accumulate during ParseTzFile() recursive calls when processing @INCLUDE
directives.
I've attached a patch with this change in case anyone thinks it's worth
applying.
Regards,
Shixin Wang
Attachments:
v1-0001-Fix-memory-leak-in-tzparser.patchapplication/octet-stream; name=v1-0001-Fix-memory-leak-in-tzparser.patchDownload
From 28663310a6a7f2459e3af633a3b1528d22b9ac06 Mon Sep 17 00:00:00 2001
From: Shixin Wang <wang-shi-xin@outlook.com>
Date: Tue, 16 Dec 2025 13:41:08 +0800
Subject: [PATCH] Fix memory leak in tzparser.c
When overriding a timezone entry in addToArray(), original
midptr->zone string was leaked before being replaced with the new
entry->zone. Add pfree() call to free the old zone string before
overwriting it.
Author: Shixin Wang <wang-shi-xin@outlook.com>
---
src/backend/utils/misc/tzparser.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/utils/misc/tzparser.c b/src/backend/utils/misc/tzparser.c
index d7e84bab981..6a1893f37ca 100644
--- a/src/backend/utils/misc/tzparser.c
+++ b/src/backend/utils/misc/tzparser.c
@@ -229,6 +229,8 @@ addToArray(tzEntry **base, int *arraysize, int n,
if (override)
{
/* same abbrev but something is different, override */
+ if (midptr->zone != NULL)
+ pfree(midptr->zone);
midptr->zone = entry->zone;
midptr->offset = entry->offset;
midptr->is_dst = entry->is_dst;
--
2.50.1 (Apple Git-155)
On Tue, Dec 16, 2025 at 05:55:32AM +0000, Shixin Wang wrote:
While the memory is managed by a temp memory context that gets cleaned up
eventually, the coarse-grained management might cause some memory to
accumulate during ParseTzFile() recursive calls when processing @INCLUDE
directives.I've attached a patch with this change in case anyone thinks it's worth
applying.
Why does it matter? load_tzoffsets() is the sole caller of
ParseTzFile() and it uses a temporary memory context named
TZParserMemory to not have to do cleanups like the one you are
proposing here.
--
Michael
On Tue, Dec 16, 2025 at 1:29 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Dec 16, 2025 at 05:55:32AM +0000, Shixin Wang wrote:
While the memory is managed by a temp memory context that gets cleaned up
eventually, the coarse-grained management might cause some memory to
accumulate during ParseTzFile() recursive calls when processing @INCLUDE
directives.I've attached a patch with this change in case anyone thinks it's worth
applying.Why does it matter? load_tzoffsets() is the sole caller of
ParseTzFile() and it uses a temporary memory context named
TZParserMemory to not have to do cleanups like the one you are
proposing here.
+1. But maybe Shixin has seen a scenario where this temporary
accumulation has caused some problems because say there were many
entries whose zone was replaced? Shixin, what problem did you see
which prompted you to create this patch?
--
Best Wishes,
Ashutosh Bapat
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:
On Tue, Dec 16, 2025 at 1:29 PM Michael Paquier <michael@paquier.xyz> wrote:
Why does it matter? load_tzoffsets() is the sole caller of
ParseTzFile() and it uses a temporary memory context named
TZParserMemory to not have to do cleanups like the one you are
proposing here.
+1. But maybe Shixin has seen a scenario where this temporary
accumulation has caused some problems because say there were many
entries whose zone was replaced? Shixin, what problem did you see
which prompted you to create this patch?
There are only several hundred timezone names in the entire world,
so it's really difficult to believe any interesting amount of
transient memory consumption here.
regards, tom lane