Date: Wed, 18 Oct 2006 21:50:26 -0500 (CDT) From: "Sean C. Farley" <sean-freebsd@farley.org> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-current@freebsd.org Subject: Re: Fix for memory leak in setenv/unsetenv Message-ID: <20061018211005.L1466@baba.farley.org> In-Reply-To: <200610111427.42195.jhb@freebsd.org> References: <20061006200320.T1063@baba.farley.org> <200610101001.04286.jhb@freebsd.org> <20061011105435.A10713@thor.farley.org> <200610111427.42195.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
--0-1563628599-1161226226=:1466
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
On Wed, 11 Oct 2006, John Baldwin wrote:
> On Wednesday 11 October 2006 12:15, Sean C. Farley wrote:
>> On Tue, 10 Oct 2006, John Baldwin wrote:
>>> This still won't work. The reason for the intentional leak is
>>> because of this code sequence:
>>>
>>> char *a;
>>>
>>> setenv("FOO", "0", 1);
>>> a = getenv("FOO");
>>> setenv("FOO", "bar", 1);
>>> printf("FOO was %s\n", a);
>>>
>>> With the memory leak fixed this will use free'd memory. While this
>>> code may seem weird in a program, it actually is quite possible for
>>> a library to read and cache the value of an environment variable.
>>> If you didn't leave the leak around, the library could cause a crash
>>> if the main program (or another library) changed the environment
>>> variable the first library had a cached pointer to the value of.
<snip>
> Yeah, but it doesn't crash is the point actually. The pointer is
> still valid, though it may be overwritten with a newer value, it's
> still valid and a library can reliably doing getenv() and that pointer
> will always point to some value of that variable, but it won't ever
> point to anything else.
<snip>
> Part of the problem is that we have no way to notify consumers of an
> environment variable when its value is changed. Alternatively, we
> could add a different variant of getenv that required the user to
> supply the buffer, but that's not the API we have.
OK. I decided to fix the memory leak as well as keep backward
compatibility. The result is on my site tar'd[1] and extracted[2]. It
still needs some touch-ups, but it works. It is even faster than the
current implementation when I compared "hungry" and "lean" (main.c
without the sleep() call).
Features:
1. No memory leak. :)
2. Able to be cleaned up: __clean_environ()
3. Backward compatible.
4. Faster.
Nice changes to make (but unnecessary):
1. Call __build_env() just after a process starts instead of checking
within the public env functions.
2. Call __rebuild_environ() just before uses of environ within libc
(i.e., execl(), execv(), execvP(), popen(), _init_tls()) instead of
after every getenv(), setenv(), unsetenv().
3. Call __clean_environ() at process exit. This allows for fewer leaks
when a developer is debugging his code.
Sean
1. http://www.farley.org/freebsd/tmp/setenv-4.tar.bz2
2. http://www.farley.org/freebsd/tmp/setenv-4/
--
sean-freebsd@farley.org
--0-1563628599-1161226226=:1466
Content-Type: TEXT/PLAIN; charset=US-ASCII; name=logging
Content-Transfer-Encoding: BASE64
Content-ID: <20061018215026.F1466@baba.farley.org>
Content-Description: ministat log
Content-Disposition: attachment; filename=logging
eCAuLi9odW5ncnkubG9nDQorIC4uL2xlYW4ubG9nDQorLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0rDQp8ICArICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICB8DQp8ICsrICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB8DQp8Kysr
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgIHggeCB4ICB8DQp8KysrICAgKyAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgeHh4eHh4IHh8DQp8fE1BfCAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHxfQV98
ICB8DQorLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0rDQogICAgTiAg
ICAgICAgICAgTWluICAgICAgICAgICBNYXggICAgICAgIE1lZGlhbiAgICAg
ICAgICAgQXZnICAgICAgICBTdGRkZXYNCnggIDEwICAgICAgICAgIDIuMjIg
ICAgICAgICAgMi4yOSAgICAgICAgICAyLjI1ICAgICAgICAgMi4yNTEgICAw
LjAyMTgzMjY5Nw0KKyAgMTAgICAgICAgICAgMS41MyAgICAgICAgICAxLjU5
ICAgICAgICAgMS41NDUgICAgICAgICAxLjU0NyAgIDAuMDE3MDI5Mzg2DQpE
aWZmZXJlbmNlIGF0IDk1LjAlIGNvbmZpZGVuY2UNCgktMC43MDQgKy8tIDAu
MDE4Mzk2Mw0KCS0zMS4yNzUlICsvLSAwLjgxNzI0OCUNCgkoU3R1ZGVudCdz
IHQsIHBvb2xlZCBzID0gMC4wMTk1Nzg5KQ0K
--0-1563628599-1161226226=:1466--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061018211005.L1466>