Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 7 Aug 1998 13:52:59 -0700 (PDT)
From:      Archie Cobbs <archie@whistle.com>
To:        dg@root.com
Cc:        archie@whistle.com, phk@critter.freebsd.dk, wollman@khavrinen.lcs.mit.edu, freebsd-current@FreeBSD.ORG, bde@zeta.org.au
Subject:   Re: memory leaks in libc
Message-ID:  <199808072052.NAA05035@bubba.whistle.com>
In-Reply-To: <199808072022.NAA24590@implode.root.com> from David Greenman at "Aug 7, 98 01:22:11 pm"

next in thread | previous in thread | raw e-mail | index | archive | help
David Greenman writes:
> >The attitude reflected in this statement is the exact opposite of mine,
> >which is: Yes, there is a memoryleak associated with use of this API,
> >let's fix the stupid bug and get on with it! Am I crazy for thinking
> >that?
> 
>    I wouldn't say you're crazy by thinking that, but I'm still concerned
> about programs that do a mix of setenv and private frobbing of the env.
> It seems to me that by "fixing" this memory leak, you're quite likely adding
> new bugs to existing programs.

Fair enough, that's a concern I can understand... I thought about this
for a while before when I thought about trying to fix the problem. Here's
the logic that I eventually came up with:

Algorithm
---------

 1. setenv() keeps a private list of all pointers it has obtained
    from calls to malloc() (these are pointers to individual strings
    only, not the entire environ[] array which is already kept track of).

 2. Whenver any one of these pointers is no longer to be used
    (either because the variable was overwritten or unsetenv() was
    called), we call free() to free the pointer.

Why any non-broken program will still work
------------------------------------------

 Any program is free to play with the environ[] array all it wants
 to, changing pointers, moving the array, reassigning 'environ", or
 whatever. At worst, the program will leak some memory each time it
 does this (nothing new).

 Example: program says x = malloc(), then says environ[12] = x.
 We never could free x because it wasn't malloc()'d from within
 the setenv() code.

The only thing that changes is if a program does this:

  s = getenv("FOO");
  putenv("FOO=new-and-longer-value");
  /* at this point, new putenv() will free(s), old putenv() won't */
  ...
  /* program references old value of FOO */
  printf("Old value of FOO was: %s\n", s);	/* this is a bug */

In the last line, "s" is dereferenced. With the existing code, this
will not segfault, whereas with the new code it might (if the malloc()
library unmaps the page, for example).

However, in BOTH cases the results of dereferencing "s" are
indeterminate.  Even with the old code, *s is not guaranteed to
have its old value, because putenv() may overwrite the old value
with the new value.  Any program which is trying to have determinate
behavior and does this contains a bug (in my judgement).

So it's sortof like the question about installing phkmalloc(), which
caused some buggy programs to start core dumping... do you accomodate
the old bugs or fix them?

If you can't tolerate this change in the meaning of "indeterminate"
then you won't like the fix. My intuition suggests that it's unlikely
that any program relies on this bug, since that program is already
broken. Morever, I'm closer to the "fix old bugs" approach than the
"accomodate old bugs" approach.

Anyway, that's the way I see it.. if I missed anything please comment.

-Archie

___________________________________________________________________________
Archie Cobbs   *   Whistle Communications, Inc.  *   http://www.whistle.com

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199808072052.NAA05035>