From owner-freebsd-bugs Sun Feb 1 11:57:42 1998 Return-Path: Received: (from majordom@localhost) by hub.freebsd.org (8.8.8/8.8.8) id LAA23774 for freebsd-bugs-outgoing; Sun, 1 Feb 1998 11:57:42 -0800 (PST) (envelope-from owner-freebsd-bugs@FreeBSD.ORG) Received: from base486.home.org (imdave@imdave.pr.mcs.net [205.164.3.77]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id LAA23764 for ; Sun, 1 Feb 1998 11:57:38 -0800 (PST) (envelope-from imdave@mcs.net) Received: (from imdave@localhost) by base486.home.org (8.8.8/8.8.8) id NAA02253; Sun, 1 Feb 1998 13:57:29 -0600 (CST) Date: Sun, 1 Feb 1998 13:57:29 -0600 (CST) From: Dave Bodenstab Message-Id: <199802011957.NAA02253@base486.home.org> To: archie@whistle.com Subject: Re: bin/5604: memory leak and other bugs in setenv(3) Cc: freebsd-bugs@FreeBSD.ORG Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org X-To-Unsubscribe: mail to majordomo@FreeBSD.org "unsubscribe freebsd-bugs" > From: Archie Cobbs > > >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 #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