Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 1 Dec 2009 05:04:31 +0000 (UTC)
From:      Brian Feldman <green@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r199983 - in head: lib/libc/stdlib tools/regression/environ
Message-ID:  <200912010504.nB154VnS053167@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: green
Date: Tue Dec  1 05:04:31 2009
New Revision: 199983
URL: http://svn.freebsd.org/changeset/base/199983

Log:
  Do not gratuitously fail *env(3) operations due to corrupt ('='-less)
  **environ entries.  This puts non-getenv(3) operations in line with
  getenv(3) in that bad environ entries do not cause all operations to
  fail.  There is still some inconsistency in that getenv(3) in the
  absence of any environment-modifying operation does not emit corrupt
  environ entry warnings.
  
  I also fixed another inconsistency in getenv(3) where updating the
  global environ pointer would not be reflected in the return values.
  It would have taken an intermediary setenv(3)/putenv(3)/unsetenv(3)
  in order to see the change.

Modified:
  head/lib/libc/stdlib/getenv.c
  head/tools/regression/environ/Makefile.envctl
  head/tools/regression/environ/envctl.c
  head/tools/regression/environ/envtest.t

Modified: head/lib/libc/stdlib/getenv.c
==============================================================================
--- head/lib/libc/stdlib/getenv.c	Tue Dec  1 03:52:50 2009	(r199982)
+++ head/lib/libc/stdlib/getenv.c	Tue Dec  1 05:04:31 2009	(r199983)
@@ -96,6 +96,8 @@ static int envVarsTotal = 0;
 
 /* Deinitialization of new environment. */
 static void __attribute__ ((destructor)) __clean_env_destructor(void);
+/* Resetting the environ pointer will affect the env functions. */
+static int __merge_environ(void);
 
 
 /*
@@ -190,6 +192,9 @@ __findenv_environ(const char *name, size
 {
 	int envNdx;
 
+	if (environ == NULL)
+		return (NULL);
+
 	/* Find variable within environ. */
 	for (envNdx = 0; environ[envNdx] != NULL; envNdx++)
 		if (strncmpeq(environ[envNdx], name, nameLen))
@@ -328,6 +333,7 @@ __build_env(void)
 {
 	char **env;
 	int activeNdx;
+	int checking;
 	int envNdx;
 	int savedErrno;
 	size_t nameLen;
@@ -339,6 +345,23 @@ __build_env(void)
 	/* Count environment variables. */
 	for (env = environ, envVarsTotal = 0; *env != NULL; env++)
 		envVarsTotal++;
+	/* Remove any corrupt variable entries, but do not error out. */
+	checking = 0;
+	while (checking < envVarsTotal) {
+		if (strchr(environ[checking], '=') != NULL) {
+			checking++;
+		} else {
+			__env_warnx(CorruptEnvValueMsg,
+			    environ[checking], strlen(environ[checking]));
+			/*
+			 * Pull back all remaining entries from checking + 1
+			 * through envVarsTotal, including the NULL at the end.
+			 */
+			memmove(&environ[checking], &environ[checking + 1],
+			    sizeof(char *) * (envVarsTotal - checking));
+			envVarsTotal--;
+		}
+	}
 	envVarsSize = envVarsTotal * 2;
 
 	/* Create new environment. */
@@ -353,18 +376,8 @@ __build_env(void)
 		    strdup(environ[envVarsTotal - envNdx - 1]);
 		if (envVars[envNdx].name == NULL)
 			goto Failure;
-		envVars[envNdx].value = strchr(envVars[envNdx].name, '=');
-		if (envVars[envNdx].value != NULL) {
-			envVars[envNdx].value++;
-			envVars[envNdx].valueSize =
-			    strlen(envVars[envNdx].value);
-		} else {
-			__env_warnx(CorruptEnvValueMsg, envVars[envNdx].name,
-			    strlen(envVars[envNdx].name));
-			errno = EFAULT;
-			goto Failure;
-		}
-
+		envVars[envNdx].value = strchr(envVars[envNdx].name, '=') + 1;
+		envVars[envNdx].valueSize = strlen(envVars[envNdx].value);
 		/*
 		 * Find most current version of variable to make active.  This
 		 * will prevent multiple active variables from being created
@@ -426,22 +439,18 @@ getenv(const char *name)
 	}
 
 	/*
-	 * An empty environment (environ or its first value) regardless if
-	 * environ has been copied before will return a NULL.
-	 *
-	 * If the environment is not empty, find an environment variable via
-	 * environ if environ has not been copied via an *env() call or been
-	 * replaced by a running program, otherwise, use the rebuilt
-	 * environment.
+	 * If we have not already allocated memory by performing
+	 * write operations on the environment, avoid doing so now.
 	 */
-	if (environ == NULL || environ[0] == NULL)
-		return (NULL);
-	else if (envVars == NULL || environ != intEnviron)
+	if (envVars == NULL)
 		return (__findenv_environ(name, nameLen));
-	else {
-		envNdx = envVarsTotal - 1;
-		return (__findenv(name, nameLen, &envNdx, true));
-	}
+
+	/* Synchronize environment. */
+	if (__merge_environ() == -1)
+		return (NULL);
+
+	envNdx = envVarsTotal - 1;
+	return (__findenv(name, nameLen, &envNdx, true));
 }
 
 
@@ -559,8 +568,7 @@ __merge_environ(void)
 				if ((equals = strchr(*env, '=')) == NULL) {
 					__env_warnx(CorruptEnvValueMsg, *env,
 					    strlen(*env));
-					errno = EFAULT;
-					return (-1);
+					continue;
 				}
 				if (__setenv(*env, equals - *env, equals + 1,
 				    1) == -1)

Modified: head/tools/regression/environ/Makefile.envctl
==============================================================================
--- head/tools/regression/environ/Makefile.envctl	Tue Dec  1 03:52:50 2009	(r199982)
+++ head/tools/regression/environ/Makefile.envctl	Tue Dec  1 05:04:31 2009	(r199983)
@@ -13,4 +13,4 @@ NO_MAN=	yes
 .include <bsd.prog.mk>
 
 test: ${PROG}
-	@sh envtest.t
+	@env -i sh envtest.t

Modified: head/tools/regression/environ/envctl.c
==============================================================================
--- head/tools/regression/environ/envctl.c	Tue Dec  1 03:52:50 2009	(r199982)
+++ head/tools/regression/environ/envctl.c	Tue Dec  1 05:04:31 2009	(r199983)
@@ -60,7 +60,7 @@ dump_environ(void)
 static void
 usage(const char *program)
 {
-	fprintf(stderr, "Usage:  %s [-DGUchrt] [-c 1|2|3|4] [-gu name] "
+	fprintf(stderr, "Usage:  %s [-DGUchrt] [-c 1|2|3|4] [-bgu name] "
 	    "[-p name=value]\n"
 	    "\t[(-S|-s name) value overwrite]\n\n"
 	    "Options:\n"
@@ -68,6 +68,7 @@ usage(const char *program)
 	    "  -G name\t\t\tgetenv(NULL)\n"
 	    "  -S value overwrite\t\tsetenv(NULL, value, overwrite)\n"
 	    "  -U\t\t\t\tunsetenv(NULL)\n"
+	    "  -b name\t\t\tblank the 'name=$name' entry, corrupting it\n"
 	    "  -c 1|2|3|4\t\t\tClear environ variable using method:\n"
 	    "\t\t\t\t1 - set environ to NULL pointer\n"
 	    "\t\t\t\t2 - set environ[0] to NULL pointer\n"
@@ -98,6 +99,28 @@ print_rtrn_errno(int rtrnVal, const char
 	return;
 }
 
+static void
+blank_env(const char *var)
+{
+	char **newenviron;
+	int n, varlen;
+
+	if (environ == NULL)
+		return;
+
+	for (n = 0; environ[n] != NULL; n++)
+		;
+	newenviron = malloc(sizeof(char *) * (n + 1));
+	varlen = strlen(var);
+	for (; n >= 0; n--) {
+		newenviron[n] = environ[n];
+		if (newenviron[n] != NULL &&
+		    strncmp(newenviron[n], var, varlen) == 0 &&
+		    newenviron[n][varlen] == '=')
+			newenviron[n] += strlen(newenviron[n]);
+	}
+	environ = newenviron;
+}
 
 int
 main(int argc, char **argv)
@@ -114,8 +137,12 @@ main(int argc, char **argv)
 	}
 
 	/* The entire program is basically executed from this loop. */
-	while ((arg = getopt(argc, argv, "DGS:Uc:g:hp:rs:tu:")) != -1) {
+	while ((arg = getopt(argc, argv, "DGS:Ub:c:g:hp:rs:tu:")) != -1) {
 		switch (arg) {
+		case 'b':
+			blank_env(optarg);
+			break;
+
 		case 'c':
 			switch (atoi(optarg)) {
 			case 1:

Modified: head/tools/regression/environ/envtest.t
==============================================================================
--- head/tools/regression/environ/envtest.t	Tue Dec  1 03:52:50 2009	(r199982)
+++ head/tools/regression/environ/envtest.t	Tue Dec  1 05:04:31 2009	(r199983)
@@ -232,3 +232,18 @@ check_result "${BAR} 0 0 ${BAR} 0 0 ${NU
 
 run_test -r -g FOO -u FOO -g FOO -s FOO ${BAR} 1 -g FOO
 check_result "${BAR} 0 0 ${NULL} 0 0 ${BAR}"
+
+
+# corruption (blanking) of environ members.
+export BLANK_ME=
+export AFTER_BLANK=blanked
+run_test -b BLANK_ME -p MORE=vars -g FOO -g BLANK_ME -g AFTER_BLANK
+check_result "0 0 ${FOO} ${NULL} ${AFTER_BLANK}"
+
+run_test -b BLANK_ME -u FOO -g FOO -g AFTER_BLANK
+check_result "0 0 ${NULL} ${AFTER_BLANK}"
+
+export BLANK_ME2=
+export AFTER_BLANKS=blankD
+run_test -b BLANK_ME -b AFTER_BLANK -b BLANK_ME2 -g FOO -g AFTER_BLANKS
+check_result "${FOO} ${AFTER_BLANKS}"



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