Date: Wed, 5 Jul 2006 20:52:40 -0500 (CDT) From: Sean Farley <sean-freebsd@farley.org> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/99826: setenv()/unsetenv() leak memory Message-ID: <200607060152.k661qeq3052993@thor.farley.org> Resent-Message-ID: <200607060200.k6620WrY039664@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 99826 >Category: bin >Synopsis: setenv()/unsetenv() leak memory >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Jul 06 02:00:32 GMT 2006 >Closed-Date: >Last-Modified: >Originator: Sean Farley >Release: FreeBSD 6.1-STABLE i386 >Organization: >Environment: System: FreeBSD thor.farley.org 6.1-STABLE FreeBSD 6.1-STABLE #0: Tue Jun 6 12:54:08 CDT 2006 root@thor.farley.org:/usr/obj/usr/src/sys/THOR i386 >Description: When writing code to use a third-party library, I ran into a leak that setenv() exhibits when overwriting a variable with a larger value. Calling unsetenv() will just leak memory. All of this is documented in the man page (setenv(3)): Successive calls to setenv() or putenv() assigning a differently sized value to the same name will result in a memory leak. The FreeBSD seman- tics for these functions (namely, that the contents of value are copied and that old values remain accessible indefinitely) make this bug unavoidable. Future versions may eliminate one or both of these semantic guarantees in order to fix the bug. Unfortunately, I must reset an environment variable quite a lot resulting in a quick memory leak. To prevent the leak, I have written a patch which copies the entire environment into dynamic memory upon the first setenv(). This way any time an existing variable has its value updated with a larger value the old value is freed and replaced with a new copy. unsetenv() will free values if setenv() has been called first. This was tested with dmalloc and MALLOC_OPTIONS=AX. A test program can be found here[1] and expanded here[2]. 1. http://www.farley.org/freebsd/tmp/setenv-3.tar.bz2 2. http://www.farley.org/freebsd/tmp/setenv-3/ >How-To-Repeat: Run hungry and watch it grow via top. Run lean and watch it hold steady. >Fix: It can be also found at http://www.farley.org/freebsd/tmp/setenv-3/setenv.c.diff -----Begin Fix----- --- /usr/src/lib/libc/stdlib/setenv.c Fri Mar 22 15:53:10 2002 +++ setenv.c Wed Jul 5 20:47:35 2006 @@ -40,9 +40,12 @@ #include <stddef.h> #include <stdlib.h> #include <string.h> +#include <sys/types.h> char *__findenv(const char *, int *); +static uint8_t dynamicEnv = 0; /* Env vars were copied */ + /* * setenv -- * Set the value of the environmental variable "name" to be @@ -59,6 +62,18 @@ char *c; int l_value, offset; + /* + * Make copies of environment variables to allow for + * reallocation. This stops a potential memory leak + * when resetting a variable with larger values. + */ + if (!dynamicEnv) { + for (offset = 0; environ[offset]; offset++) + if ((environ[offset] = strdup(environ[offset])) == NULL) + return (-1); + dynamicEnv = 1; + } + if (*value == '=') /* no `=' in value */ ++value; l_value = strlen(value); @@ -74,27 +89,21 @@ char **p; for (p = environ, cnt = 0; *p; ++p, ++cnt); - if (alloced == environ) { /* just increase size */ - p = (char **)realloc((char *)environ, - (size_t)(sizeof(char *) * (cnt + 2))); - if (!p) - return (-1); - alloced = environ = p; - } - else { /* get new space */ - /* copy old entries into it */ - p = malloc((size_t)(sizeof(char *) * (cnt + 2))); - if (!p) - return (-1); + p = (char **)realloc((char *)alloced, + (size_t)(sizeof(char *) * (cnt + 2))); + if (!p) + return (-1); + if (alloced != environ) { /* alloced environ */ + /* copy old entries into it */ bcopy(environ, p, cnt * sizeof(char *)); - alloced = environ = p; } + alloced = environ = p; environ[cnt + 1] = NULL; offset = cnt; } for (c = (char *)name; *c && *c != '='; ++c); /* no `=' in name */ if (!(environ[offset] = /* name + `=' + value */ - malloc((size_t)((int)(c - name) + l_value + 2)))) + reallocf(environ[offset], (size_t)((int)(c - name) + l_value + 2)))) return (-1); for (c = environ[offset]; (*c = *name++) && *c != '='; ++c); for (*c++ = '='; (*c++ = *value++); ); @@ -113,8 +122,11 @@ char **p; int offset; - while (__findenv(name, &offset)) /* if set multiple times */ + while (__findenv(name, &offset)) { /* if set multiple times */ + if (dynamicEnv) + free(environ[offset]); for (p = &environ[offset];; ++p) if (!(*p = *(p + 1))) break; + } } -----End Fix----- >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200607060152.k661qeq3052993>