* [musl] [PATCH] time/tz: Fix memory leak when do_tzset() is repeated
@ 2023-05-09 12:44 Xiaoming Ni
2023-05-09 14:54 ` Rich Felker
0 siblings, 1 reply; 2+ messages in thread
From: Xiaoming Ni @ 2023-05-09 12:44 UTC (permalink / raw)
To: dalias, musl; +Cc: nixiaoming, liucheng32
When do_tzset() and setenv("TZ") are repeatedly called,
"old_tz" is repeatedly applied for memory and is not released,
triggering memory leakage.
s = getenv("TZ");
i = strlen(s);
if (i >= old_tz_size) {
old_tz = malloc(old_tz_size);// without free old value
}
add free(old_tz) to avoid memory leak.
Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com>
---
src/time/__tz.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/time/__tz.c b/src/time/__tz.c
index c34b3eb7..917740ea 100644
--- a/src/time/__tz.c
+++ b/src/time/__tz.c
@@ -151,6 +151,7 @@ static void do_tzset()
old_tz_size *= 2;
if (i >= old_tz_size) old_tz_size = i+1;
if (old_tz_size > PATH_MAX+2) old_tz_size = PATH_MAX+2;
+ if (old_tz != old_tz_buf) free(old_tz);
old_tz = malloc(old_tz_size);
}
if (old_tz) memcpy(old_tz, s, i+1);
--
2.27.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [musl] [PATCH] time/tz: Fix memory leak when do_tzset() is repeated
2023-05-09 12:44 [musl] [PATCH] time/tz: Fix memory leak when do_tzset() is repeated Xiaoming Ni
@ 2023-05-09 14:54 ` Rich Felker
0 siblings, 0 replies; 2+ messages in thread
From: Rich Felker @ 2023-05-09 14:54 UTC (permalink / raw)
To: Xiaoming Ni; +Cc: musl, liucheng32
On Tue, May 09, 2023 at 08:44:24PM +0800, Xiaoming Ni wrote:
> When do_tzset() and setenv("TZ") are repeatedly called,
> "old_tz" is repeatedly applied for memory and is not released,
> triggering memory leakage.
>
> s = getenv("TZ");
> i = strlen(s);
> if (i >= old_tz_size) {
> old_tz = malloc(old_tz_size);// without free old value
> }
>
> add free(old_tz) to avoid memory leak.
>
> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com>
> ---
> src/time/__tz.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/time/__tz.c b/src/time/__tz.c
> index c34b3eb7..917740ea 100644
> --- a/src/time/__tz.c
> +++ b/src/time/__tz.c
> @@ -151,6 +151,7 @@ static void do_tzset()
> old_tz_size *= 2;
> if (i >= old_tz_size) old_tz_size = i+1;
> if (old_tz_size > PATH_MAX+2) old_tz_size = PATH_MAX+2;
> + if (old_tz != old_tz_buf) free(old_tz);
> old_tz = malloc(old_tz_size);
> }
> if (old_tz) memcpy(old_tz, s, i+1);
> --
> 2.27.0
The whole purpose of the code block you're modifying is to grow the
size of the buffer geometrically so that the total allocation is well
bounded (on the order of 2x PATH_MAX) regardless of how many times you
change the TZ or how long a TZ string is, so that it does not have to
be freed. On top of this, a normal program that's not doing anything
unsafe (modifying own environment is unsafe without constraints like
knowing you'll always be single-threaded) only allocates once.
In general, musl avoids having libc components depend on malloc, and
when that's inevitable, avoids having them depend on free unless the
interface contract requires it. This way, only programs that actually
use malloc need to link malloc.
Rich
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-05-09 14:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-09 12:44 [musl] [PATCH] time/tz: Fix memory leak when do_tzset() is repeated Xiaoming Ni
2023-05-09 14:54 ` Rich Felker
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).