Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230509145455.GK4163@brightrain.aerifal.cx>
Date: Tue, 9 May 2023 10:54:55 -0400
From: Rich Felker <dalias@...ifal.cx>
To: Xiaoming Ni <nixiaoming@...wei.com>
Cc: musl@...ts.openwall.com, liucheng32@...wei.com
Subject: Re: [PATCH] time/tz: Fix memory leak when do_tzset() is
 repeated

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@...wei.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

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.