From patchwork Wed Mar 11 18:48:00 2015
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Paul Eggert <eggert@cs.ucla.edu>
X-Patchwork-Id: 5588
Received: (qmail 5331 invoked by alias); 11 Mar 2015 18:48:07 -0000
Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <libc-alpha.sourceware.org>
List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org>
List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/libc-alpha/>
List-Post: <mailto:libc-alpha@sourceware.org>
List-Help: <mailto:libc-alpha-help@sourceware.org>,
	<http://sourceware.org/ml/#faqs>
Sender: libc-alpha-owner@sourceware.org
Delivered-To: mailing list libc-alpha@sourceware.org
Received: (qmail 5173 invoked by uid 89); 11 Mar 2015 18:48:05 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL, BAYES_00,
	SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2
X-HELO: smtp.cs.ucla.edu
Message-ID: <55008DE0.8050805@cs.ucla.edu>
Date: Wed, 11 Mar 2015 11:48:00 -0700
From: Paul Eggert <eggert@cs.ucla.edu>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
	rv:31.0) Gecko/20100101 Thunderbird/31.5.0
MIME-Version: 1.0
To: Paul Pluzhnikov <ppluzhnikov@google.com>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: Joseph Myers <joseph@codesourcery.com>,
	GLIBC Devel <libc-alpha@sourceware.org>,
	Roland McGrath <roland@hack.frob.com>, "mtk@man7.org" <mtk@man7.org>
Subject: Re: [patch] Error on setenv(..., NULL, ...)
References: 
 <CALoOobNSbWUkd_i-L6U0ovbqPYnJY-h=ftX1K61yb19pmJj6aw@mail.gmail.com>
	<alpine.DEB.2.10.1503111712240.30954@digraph.polyomino.org.uk>
	<CALoOobPKxfJfnbcUKH8osgCZMeSiD83K1OiF+_vSeAy0ewe1Jw@mail.gmail.com>
	<55008721.1090200@arm.com>
	<CALoOobNbOgm5=oFbEUmTbca3M-KqSUgGmTeWYOt1zTN-CTLoog@mail.gmail.com>
In-Reply-To: 
 <CALoOobNbOgm5=oFbEUmTbca3M-KqSUgGmTeWYOt1zTN-CTLoog@mail.gmail.com>

On 03/11/2015 11:26 AM, Paul Pluzhnikov wrote:
> Where does it say that NULL name is allowed?
It doesn't.  But that's the FreeBSD behavior.

FreeBSD setenv (..., NULL, ...) dumps core quickly because it calls 
strlen (NULL).  How about if we do the same?  It should be just as fast 
as what we do now, and it's safer and more compatible. Something like 
the attached untested patch, say.

diff --git a/ChangeLog b/ChangeLog
index 7360079..b9b40c9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2015-03-11  Paul Eggert  <eggert@cs.ucla.edu>
+
+	* stdlib/setenv.c (__add_to_environ):
+	Dump core quickly if setenv (..., NULL, ...) is called.
+
 2015-03-11  Paul Pluzhnikov  <ppluzhnikov@google.com>
 
 	[BZ #18043]
diff --git a/stdlib/setenv.c b/stdlib/setenv.c
index b60c4f0..f3de7e9 100644
--- a/stdlib/setenv.c
+++ b/stdlib/setenv.c
@@ -115,7 +115,13 @@ __add_to_environ (name, value, combined, replace)
   char **ep;
   size_t size;
   const size_t namelen = strlen (name);
-  const size_t vallen = value != NULL ? strlen (value) + 1 : 0;
+  size_t vallen;
+
+  /* Test COMBINED, not VALUE, since VALLEN is needed only if COMBINED
+     is non-null.  Also, testing COMBINED causes setenv (..., NULL, ...)
+     to dump core quickly instead of corrupting memory.  */
+  if (combined != NULL)
+    vallen = strlen (value) + 1;
 
   LOCK;