diff mbox

Error on setenv(..., NULL, ...)

Message ID 55008DE0.8050805@cs.ucla.edu
State Superseded
Headers

Commit Message

Paul Eggert March 11, 2015, 6:48 p.m. UTC 
  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.

Comments

Roland McGrath March 12, 2015, 9:53 p.m. UTC | #1 
I like Eggert's approach.  Off hand it seems it would be better to simply
move the definition of VALLEN into the scope containing its uses, which is
the else branch of 'if (combined != NULL)'.  The only reason I can see not
to do that is to force the fault to be before the lock is taken.  If that
is the explicit intent of the code, then its comments should say so.
diff mbox

Patch

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;