From owner-svn-src-all@FreeBSD.ORG  Wed Dec 18 17:03:44 2013
Return-Path: <owner-svn-src-all@FreeBSD.ORG>
Delivered-To: svn-src-all@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org
 [IPv6:2001:1900:2254:206a::19:1])
 (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by hub.freebsd.org (Postfix) with ESMTPS id 59611443;
 Wed, 18 Dec 2013 17:03:44 +0000 (UTC)
Received: from svn.freebsd.org (svn.freebsd.org
 [IPv6:2001:1900:2254:2068::e6a:0])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by mx1.freebsd.org (Postfix) with ESMTPS id 2B7D71FD9;
 Wed, 18 Dec 2013 17:03:44 +0000 (UTC)
Received: from svn.freebsd.org ([127.0.1.70])
 by svn.freebsd.org (8.14.7/8.14.7) with ESMTP id rBIH3i6Z085124;
 Wed, 18 Dec 2013 17:03:44 GMT (envelope-from marcel@svn.freebsd.org)
Received: (from marcel@localhost)
 by svn.freebsd.org (8.14.7/8.14.7/Submit) id rBIH3iEb085123;
 Wed, 18 Dec 2013 17:03:44 GMT (envelope-from marcel@svn.freebsd.org)
Message-Id: <201312181703.rBIH3iEb085123@svn.freebsd.org>
From: Marcel Moolenaar <marcel@FreeBSD.org>
Date: Wed, 18 Dec 2013 17:03:44 +0000 (UTC)
To: src-committers@freebsd.org, svn-src-all@freebsd.org,
 svn-src-head@freebsd.org
Subject: svn commit: r259561 - head/lib/libstand
X-SVN-Group: head
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-BeenThere: svn-src-all@freebsd.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: "SVN commit messages for the entire src tree \(except for &quot;
 user&quot; and &quot; projects&quot; \)" <svn-src-all.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/svn-src-all>,
 <mailto:svn-src-all-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-all/>
List-Post: <mailto:svn-src-all@freebsd.org>
List-Help: <mailto:svn-src-all-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-all>,
 <mailto:svn-src-all-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 18 Dec 2013 17:03:44 -0000

Author: marcel
Date: Wed Dec 18 17:03:43 2013
New Revision: 259561
URL: http://svnweb.freebsd.org/changeset/base/259561

Log:
  Fix an inappropriate free of a non-dynamic value. While here, make the
  code more naive and robust:
  1.  When setting ev_value, also always set ev_flags appropriately
  2.  Always check ev_value and ev_flags before calling free.
  
  Both the value and the EV_DYNAMIC property can come directly from the
  consumers of the environment functionality, so it's good to be careful.
  And since this code is typically not looked at for long periods of
  time, it's good to have it be a little "dumb-looking".
  
  Trigger case for the bug:
          env_setenv("foo", 0, "1", NULL, NULL);
          env_setenv("foo", 0, "2", NULL, NULL);
  
  Obtained from:	Juniper Networks, Inc.

Modified:
  head/lib/libstand/environment.c

Modified: head/lib/libstand/environment.c
==============================================================================
--- head/lib/libstand/environment.c	Wed Dec 18 16:14:35 2013	(r259560)
+++ head/lib/libstand/environment.c	Wed Dec 18 17:03:43 2013	(r259561)
@@ -75,7 +75,14 @@ env_setenv(const char *name, int flags, 
 	 * for one already.
 	 */
 	if ((ev->ev_sethook != NULL) && !(flags & EV_NOHOOK))
-	    return(ev->ev_sethook(ev, flags, value));
+	    return (ev->ev_sethook(ev, flags, value));
+
+	/* If there is data in the variable, discard it. */
+	if (ev->ev_value != NULL && (ev->ev_flags & EV_DYNAMIC) != 0)
+	    free(ev->ev_value);
+	ev->ev_value = NULL;
+	ev->ev_flags &= ~EV_DYNAMIC;
+
     } else {
 
 	/*
@@ -84,6 +91,7 @@ env_setenv(const char *name, int flags, 
 	ev = malloc(sizeof(struct env_var));
 	ev->ev_name = strdup(name);
 	ev->ev_value = NULL;
+	ev->ev_flags = 0;
 	/* hooks can only be set when the variable is instantiated */
 	ev->ev_sethook = sethook;
 	ev->ev_unsethook = unsethook;
@@ -117,21 +125,16 @@ env_setenv(const char *name, int flags, 
 	    }
 	}
     }
-    
-    /* If there is data in the variable, discard it */
-    if (ev->ev_value != NULL)
-	free(ev->ev_value);
 
     /* If we have a new value, use it */
     if (flags & EV_VOLATILE) {
 	ev->ev_value = strdup(value);
+	ev->ev_flags |= EV_DYNAMIC;
     } else {
 	ev->ev_value = (char *)value;
+	ev->ev_flags |= flags & EV_DYNAMIC;
     }
 
-    /* Keep the flag components that are relevant */
-    ev->ev_flags = flags & (EV_DYNAMIC);
-
     return(0);
 }
 
@@ -201,7 +204,7 @@ env_discard(struct env_var *ev)
     if (environ == ev)
 	environ = ev->ev_next;
     free(ev->ev_name);
-    if (ev->ev_flags & EV_DYNAMIC)
+    if (ev->ev_value != NULL && (ev->ev_flags & EV_DYNAMIC) != 0)
 	free(ev->ev_value);
     free(ev);
 }