Date: Sun, 1 Feb 1998 13:57:29 -0600 (CST) From: Dave Bodenstab <imdave@mcs.net> To: archie@whistle.com Cc: freebsd-bugs@FreeBSD.ORG Subject: Re: bin/5604: memory leak and other bugs in setenv(3) Message-ID: <199802011957.NAA02253@base486.home.org>
next in thread | raw e-mail | index | archive | help
> From: Archie Cobbs <archie@whistle.com>
>
> >Description:
>
> There is a memory leak in the setenv() function. If you overwrite
> a value with a longer value, the memory allocated for the shorter
> value is never freed.
This is ``the way it is'' as long as I can recall (from system 5
release 2 in the early 80's.) This is because the initial environment
values and environ[] array are created by the kernel when a process's
address space is created by the exec(2) system call. Take a look
at /usr/src/libc/csu/i386/crt0.c and /usr/src/sys/kern/kern_exec.c.
These areas are not on malloc's memory lists, therefore it is
illegal to call free with any of these addresses. Unless setenv
were changed to keep a record of which environ[] elements had been
malloc'ed by a previous call to setenv, there is no way to know if
it is OK to call free(). Your fix to setenv makes an illegal call
to free -- change your test program to:
#include <stdlib.h>
#define BSIZE 1024
char buf[BSIZE + 1];
int main(int ac, char *av[])
{
int x;
memset(buf, 'b', BSIZE);
buf[BSIZE] = 0;
for (x = 0; 1; x++)
{
buf[x % BSIZE] = 0;
setenv("PATH", buf, 1);
buf[x % BSIZE] = 'b';
}
return(0);
}
Compile with your patched setenv.c and run with:
bash$ MALLOC_OPTIONS=AZX gdb ./a.out
GDB is free software and you are welcome to distribute copies of it
under certain conditions; type "show copying" to see the conditions.
There is absolutely no warranty for GDB; type "show warranty" for details.
GDB 4.16 (i386-unknown-freebsd),
Copyright 1996 Free Software Foundation, Inc...
(gdb) r
Starting program: /tmp/./a.out
a.out in free(): error: junk pointer, too high to make sense.
Program received signal SIGABRT, Aborted.
0x806c571 in kill ()
(gdb) bt
#0 0x806c571 in kill ()
#1 0x806bde3 in abort ()
#2 0x806a862 in getdtablesize ()
#3 0x806a8a0 in getdtablesize ()
#4 0x806b8a3 in getdtablesize ()
#5 0x806bad6 in free ()
#6 0x1843 in setenv (name=0x15b0 "PATH", value=0x20f8 "b", rewrite=1)
at setenv.c:97
#7 0x161f in main (ac=1, av=0xefbfd934) at t.c:12
(gdb) f 6
#6 0x1843 in setenv (name=0x15b0 "PATH", value=0x20f8 "b", rewrite=1)
at setenv.c:97
97 free(environ[offset]);
(gdb) q
The program is running. Quit anyway (and kill it)? (y or n) y
I suspect that the original designers of setenv made a trade off
between a more complicated setenv and a minor memory leak. I'm
sure that they never considered a program such as your test program
as a typical use of setenv. Whether this is a valid assumption or
not could certainly be an issue for discussion.
> Also, notice what happens to "environ" in the original code when
> the realloc() function call fails.
>
> Also, the "alloced" flag is incorrectly set if the original malloc()
> fails.
In my opinion, these two points are valid.
Dave Bodenstab
imdave@mcs.net
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199802011957.NAA02253>