From owner-freebsd-current@FreeBSD.ORG Wed Oct 11 16:15:40 2006 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id F214016A494; Wed, 11 Oct 2006 16:15:40 +0000 (UTC) (envelope-from sean-freebsd@farley.org) Received: from mail.farley.org (farley.org [67.64.95.201]) by mx1.FreeBSD.org (Postfix) with ESMTP id C748043D91; Wed, 11 Oct 2006 16:15:32 +0000 (GMT) (envelope-from sean-freebsd@farley.org) Received: from thor.farley.org (thor.farley.org [192.168.1.5]) by mail.farley.org (8.13.8/8.13.8) with ESMTP id k9BGGRZC004297; Wed, 11 Oct 2006 11:16:27 -0500 (CDT) (envelope-from sean-freebsd@farley.org) Date: Wed, 11 Oct 2006 11:15:26 -0500 (CDT) From: "Sean C. Farley" To: John Baldwin In-Reply-To: <200610101001.04286.jhb@freebsd.org> Message-ID: <20061011105435.A10713@thor.farley.org> References: <20061006200320.T1063@baba.farley.org> <200610101001.04286.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-current@freebsd.org Subject: Re: Fix for memory leak in setenv/unsetenv X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Oct 2006 16:15:41 -0000 On Tue, 10 Oct 2006, John Baldwin wrote: > On Friday 06 October 2006 21:13, Sean C. Farley wrote: >> Many a moon ago[1], I put together a patch to fix the leak in >> setenv() and unsetenv(). A few months ago, I submitted a PR >> (kern/99826[2]) for the final fix. I was wondering if anyone would >> take a look at it to see if any changes are still warranted. The PR >> contains information about the patch and sample programs to test it >> out. >> >> Thank you. >> >> Sean >> 1. http://lists.freebsd.org/pipermail/freebsd-hackers/2005-February/010463.html >> 2. http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/99826 > > 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. Although it would not crash, the following would fail anyway: setenv("FOO", "bar", 1); a = getenv("FOO"); setenv("FOO", "0", 1); printf("FOO was %s\n", a); In this scenario, the printf() would print "0" since the second value had a string length less than or equal to the previous value. The current implementation of setenv() would reuse the string instead of malloc'ing a new one. Also, this snippet from IEEE Std 1003.1, 2004 Edition regarding getenv()[3]: The return value from getenv() may point to static data which may be overwritten by subsequent calls to getenv(), setenv(), or unsetenv(). After the call to the second setenv(), a portable application should not assume that a still points to the same value. Also, it says "may point to static data" suggesting (at least to me) that the pointer may point to dynamic memory and be freed following the call to setenv(). > I know for one app at my last job we had a problem with this with TZ, > and so we explicitly space padded the timezone name out to a > fixed-size each time to avoid the leak. This is what I am trying to fix in setenv(). :) Sean 3. http://www.opengroup.org/onlinepubs/009695399/functions/getenv.html -- sean-freebsd@farley.org