From 9bab236702b4f33a990011b08a2deebbb8ea6e0c Mon Sep 17 00:00:00 2001 From: Sean Farley Date: Fri, 20 Jul 2007 23:30:13 +0000 Subject: [PATCH] Added environ-replacement detection. For programs that "clean" (i.e., su) or replace (i.e., zdump) the environment after a call to setenv(), putenv() or unsetenv() has been made, a few changes were made. - getenv() will return the value from the new environ array. - setenv() was split into two functions: __setenv() which is most of the previous setenv() without checks on the name and setenv() which contains the checks before calling __setenv(). - setenv(), putenv() and unsetenv() will unset all previous values and call __setenv() on all entries in the new environ array which in turn adds them to the end of the envVars array. Calling __setenv() instead of setenv() is done to avoid the temporary replacement of the '=' in a string with a NUL byte. Some strings may be read-only data. Added more regression checks for clearing the environment array. Replaced gettimeofday() with getrusage() in timing regression check for better accuracy. Fixed an off-by-one bug in __remove_putenv() in the use of memmove(). This went unnoticed due to the allocation of double the number of environ entries when building envVars. Fixed a few spelling mistakes in the comments. Reviewed by: ache Approved by: wes Approved by: re (kensmith) --- lib/libc/stdlib/getenv.c | 241 +++++++++++++++++++---------- tools/regression/environ/envctl.c | 26 +++- tools/regression/environ/envtest.t | 21 +++ tools/regression/environ/timings.c | 57 +++---- 4 files changed, 233 insertions(+), 112 deletions(-) diff --git a/lib/libc/stdlib/getenv.c b/lib/libc/stdlib/getenv.c index 5f6f49797362..b6f612181132 100644 --- a/lib/libc/stdlib/getenv.c +++ b/lib/libc/stdlib/getenv.c @@ -36,6 +36,12 @@ __FBSDID("$FreeBSD$"); +static const char CorruptEnvFindMsg[] = + "environment corrupt; unable to find %.*s"; +static const char CorruptEnvValueMsg[] = + "environment corrupt; missing value for %s"; + + /* * Standard environ. environ variable is exposed to entire process. * @@ -43,9 +49,12 @@ __FBSDID("$FreeBSD$"); * allows environ to return to as it was before. * environSize: Number of variables environ can hold. Can only * increase. + * intEnviron: Internally-built environ. Exposed via environ during + * (re)builds of the environment. */ extern char **environ; static char **origEnviron; +static char **intEnviron = NULL; static int environSize = 0; /* @@ -84,7 +93,7 @@ static int envVarsTotal = 0; /* Deinitialization of new environment. */ -static void __attribute__ ((destructor)) __clean_env(void); +static void __attribute__ ((destructor)) __clean_env_destructor(void); /* @@ -172,6 +181,64 @@ __findenv_environ(const char *name, size_t nameLen) } +/* + * Remove variable added by putenv() from variable tracking array. + */ +static void +__remove_putenv(int envNdx) +{ + envVarsTotal--; + if (envVarsTotal > envNdx) + memmove(&(envVars[envNdx]), &(envVars[envNdx + 1]), + (envVarsTotal - envNdx) * sizeof (*envVars)); + memset(&(envVars[envVarsTotal]), 0, sizeof (*envVars)); + + return; +} + + +/* + * Deallocate the environment built from environ as well as environ then set + * both to NULL. Eases debugging of memory leaks. + */ +static void +__clean_env(bool freeVars) +{ + int envNdx; + + /* Deallocate environment and environ if created by *env(). */ + if (envVars != NULL) { + for (envNdx = envVarsTotal - 1; envNdx >= 0; envNdx--) + /* Free variables or deactivate them. */ + if (envVars[envNdx].putenv) { + if (!freeVars) + __remove_putenv(envNdx); + } else { + if (freeVars) + free(envVars[envNdx].name); + else + envVars[envNdx].active = false; + } + if (freeVars) { + free(envVars); + envVars = NULL; + } else + envActive = 0; + + /* Restore original environ if it has not updated by program. */ + if (origEnviron != NULL) { + if (environ == intEnviron) + environ = origEnviron; + free(intEnviron); + intEnviron = NULL; + environSize = 0; + } + } + + return; +} + + /* * Using the environment, rebuild the environ array for use by other C library * calls that depend upon it. @@ -187,20 +254,23 @@ __rebuild_environ(int newEnvironSize) /* Resize environ. */ if (newEnvironSize > environSize) { tmpEnvironSize = newEnvironSize * 2; - tmpEnviron = realloc(environ, sizeof (*environ) * + tmpEnviron = realloc(intEnviron, sizeof (*intEnviron) * (tmpEnvironSize + 1)); if (tmpEnviron == NULL) return (-1); environSize = tmpEnvironSize; - environ = tmpEnviron; + intEnviron = tmpEnviron; } envActive = newEnvironSize; /* Assign active variables to environ. */ for (envNdx = envVarsTotal - 1, environNdx = 0; envNdx >= 0; envNdx--) if (envVars[envNdx].active) - environ[environNdx++] = envVars[envNdx].name; - environ[environNdx] = NULL; + intEnviron[environNdx++] = envVars[envNdx].name; + intEnviron[environNdx] = NULL; + + /* Always set environ which may have been replaced by program. */ + environ = intEnviron; return (0); } @@ -241,15 +311,12 @@ __build_env(void) char **env; int activeNdx; int envNdx; - int rtrnVal; int savedErrno; size_t nameLen; /* Check for non-existant environment. */ - if (environ == NULL) + if (environ == NULL || environ[0] == NULL) return (0); - if (environ[0] == NULL) - goto SaveEnviron; /* Count environment variables. */ for (env = environ, envVarsTotal = 0; *env != NULL; env++) @@ -274,8 +341,7 @@ __build_env(void) envVars[envNdx].valueSize = strlen(envVars[envNdx].value); } else { - warnx("environment corrupt; missing value for %s", - envVars[envNdx].name); + warnx(CorruptEnvValueMsg, envVars[envNdx].name); errno = EFAULT; goto Failure; } @@ -290,8 +356,7 @@ __build_env(void) activeNdx = envVarsTotal - 1; if (__findenv(envVars[envNdx].name, nameLen, &activeNdx, false) == NULL) { - warnx("environment corrupt; unable to find %.*s", - nameLen, envVars[envNdx].name); + warnx(CorruptEnvFindMsg, nameLen, envVars[envNdx].name); errno = EFAULT; goto Failure; } @@ -299,24 +364,14 @@ __build_env(void) } /* Create a new environ. */ -SaveEnviron: origEnviron = environ; environ = NULL; - if (envVarsTotal > 0) { - rtrnVal = __rebuild_environ(envVarsTotal); - if (rtrnVal == -1) { - savedErrno = errno; - __clean_env(); - errno = savedErrno; - } - } else - rtrnVal = 0; - - return (rtrnVal); + if (__rebuild_environ(envVarsTotal) == 0) + return (0); Failure: savedErrno = errno; - __clean_env(); + __clean_env(true); errno = savedErrno; return (-1); @@ -324,42 +379,12 @@ __build_env(void) /* - * Remove variable added by putenv() from variable tracking array. + * Destructor function with default argument to __clean_env(). */ static void -__remove_putenv(int envNdx) +__clean_env_destructor(void) { - memmove(&(envVars[envNdx]), &(envVars[envNdx + 1]), - (envVarsTotal - envNdx) * sizeof (*envVars)); - envVarsTotal--; - - return; -} - - -/* - * Deallocate the environment built from environ as well as environ then set - * both to NULL. Eases debugging of memory leaks. - */ -static void -__clean_env(void) -{ - int envNdx; - - /* Deallocate environment and environ if created by *env(). */ - if (envVars != NULL) { - for (envNdx = 0; envNdx < envVarsTotal; envNdx++) - if (!envVars[envNdx].putenv) - free(envVars[envNdx].name); - free(envVars); - envVars = NULL; - - /* Restore original environ. */ - if (origEnviron != NULL) { - free(environ); - environ = origEnviron; - } - } + __clean_env(true); return; } @@ -380,8 +405,12 @@ getenv(const char *name) return (NULL); } - /* Find environment variable via environ or rebuilt environment. */ - if (envVars == NULL) + /* + * Find environment variable via environ if no changes have been made + * via a *env() call or environ has been replaced by a running program, + * otherwise, use the rebuilt environment. + */ + if (envVars == NULL || environ != intEnviron) return (__findenv_environ(name, nameLen)); else { envNdx = envVarsTotal - 1; @@ -395,27 +424,19 @@ getenv(const char *name) * older setting has enough room to store the new value, it will be reused. No * previous variables are ever freed here to avoid causing a segmentation fault * in a user's code. + * + * The variables nameLen and valueLen are passed into here to allow the caller + * to calculate the length by means besides just strlen(). */ -int -setenv(const char *name, const char *value, int overwrite) +static int +__setenv(const char *name, size_t nameLen, const char *value, int overwrite) { bool reuse; char *env; int envNdx; int newEnvActive; - size_t nameLen; size_t valueLen; - /* Check for malformed name. */ - if (name == NULL || (nameLen = __strleneq(name)) == 0) { - errno = EINVAL; - return (-1); - } - - /* Initialize environment. */ - if (envVars == NULL && __build_env() == -1) - return (-1); - /* Find existing environment variable large enough to use. */ envNdx = envVarsTotal - 1; newEnvActive = envActive; @@ -437,7 +458,6 @@ setenv(const char *name, const char *value, int overwrite) /* Entry is large enough to reuse. */ else if (envVars[envNdx].valueSize >= valueLen) reuse = true; - } /* Create new variable if none was found of sufficient size. */ @@ -480,7 +500,72 @@ setenv(const char *name, const char *value, int overwrite) /* - * Insert a "name=value" string into then environment. Special settings must be + * If the program attempts to replace the array of environment variables + * (environ) environ, then deactivate all variables and merge in the new list + * from environ. + */ +static int +__merge_environ(void) +{ + char **env; + char *equals; + + /* environ has been replaced. clean up everything. */ + if (envVarsTotal > 0 && environ != intEnviron) { + /* Deactivate all environment variables. */ + if (envActive > 0) { + origEnviron = NULL; + __clean_env(false); + } + + /* + * Insert new environ into existing, yet deactivated, + * environment array. + */ + origEnviron = environ; + if (origEnviron != NULL) + for (env = origEnviron; *env != NULL; env++) { + if ((equals = strchr(*env, '=')) == NULL) { + warnx(CorruptEnvValueMsg, *env); + errno = EFAULT; + return (-1); + } + if (__setenv(*env, equals - *env, equals + 1, + 1) == -1) + return (-1); + } + } + + return (0); +} + + +/* + * The exposed setenv() that peforms a few tests before calling the function + * (__setenv()) that does the actual work of inserting a variable into the + * environment. + */ +int +setenv(const char *name, const char *value, int overwrite) +{ + size_t nameLen; + + /* Check for malformed name. */ + if (name == NULL || (nameLen = __strleneq(name)) == 0) { + errno = EINVAL; + return (-1); + } + + /* Initialize environment. */ + if (__merge_environ() == -1 || (envVars == NULL && __build_env() == -1)) + return (-1); + + return (__setenv(name, nameLen, value, overwrite)); +} + + +/* + * Insert a "name=value" string into the environment. Special settings must be * made to keep setenv() from reusing this memory block and unsetenv() from * allowing it to be tracked. */ @@ -500,7 +585,7 @@ putenv(char *string) } /* Initialize environment. */ - if (envVars == NULL && __build_env() == -1) + if (__merge_environ() == -1 || (envVars == NULL && __build_env() == -1)) return (-1); /* Deactivate previous environment variable. */ @@ -552,7 +637,7 @@ unsetenv(const char *name) } /* Initialize environment. */ - if (envVars == NULL && __build_env() == -1) + if (__merge_environ() == -1 || (envVars == NULL && __build_env() == -1)) return (-1); /* Deactivate specified variable. */ diff --git a/tools/regression/environ/envctl.c b/tools/regression/environ/envctl.c index 2681741da723..e077e42fa407 100644 --- a/tools/regression/environ/envctl.c +++ b/tools/regression/environ/envctl.c @@ -54,17 +54,19 @@ dump_environ(void) static void usage(const char *program) { - fprintf(stderr, "Usage: %s [-DGUcht] [-gu name] [-p name=value] " + fprintf(stderr, "Usage: %s [-CDGUchrt] [-gu name] [-p name=value] " "[(-S|-s name) value overwrite]\n\n" "Options:\n" + " -C\t\t\t\tClear environ variable with NULL pointer\n" " -D\t\t\t\tDump environ\n" " -G name\t\t\tgetenv(NULL)\n" " -S value overwrite\t\tsetenv(NULL, value, overwrite)\n" " -U\t\t\t\tunsetenv(NULL)\n" - " -c\t\t\t\tClear environ variable\n" + " -c\t\t\t\tClear environ variable with calloc()'d memory\n" " -g name\t\t\tgetenv(name)\n" " -h\t\t\t\tHelp\n" " -p name=value\t\t\tputenv(name=value)\n" + " -r\t\t\t\treplace environ with { \"FOO=bar\", NULL }\n" " -s name value overwrite\tsetenv(name, value, overwrite)\n" " -t\t\t\t\tOutput is suitable for testing (no newlines)\n" " -u name\t\t\tunsetenv(name)\n", @@ -77,7 +79,7 @@ usage(const char *program) int main(int argc, char **argv) { - char *cleanEnv[] = { NULL }; + char *staticEnv[] = { "FOO=bar", NULL }; char arg; const char *eol = "\n"; const char *value; @@ -87,15 +89,19 @@ main(int argc, char **argv) exit(EXIT_FAILURE); } - while ((arg = getopt(argc, argv, "DGS:Ucg:hp:s:tu:")) != -1) { + while ((arg = getopt(argc, argv, "CDGS:Ucg:hp:rs:tu:")) != -1) { switch (arg) { - case 'D': - errno = 0; - dump_environ(); + case 'C': + environ = NULL; break; case 'c': - environ = cleanEnv; + environ = calloc(1, sizeof(*environ)); + break; + + case 'D': + errno = 0; + dump_environ(); break; case 'G': @@ -113,6 +119,10 @@ main(int argc, char **argv) printf("%d %d%s", putenv(optarg), errno, eol); break; + case 'r': + environ = staticEnv; + break; + case 'S': errno = 0; printf("%d %d%s", setenv(NULL, optarg, diff --git a/tools/regression/environ/envtest.t b/tools/regression/environ/envtest.t index e3aca952e2da..2b2c548553bc 100644 --- a/tools/regression/environ/envtest.t +++ b/tools/regression/environ/envtest.t @@ -132,6 +132,9 @@ run_test -s FOO ${NEWBAR} 1 -s FOO ${BAR} 1 -s FOO ${NEWBAR} 1 -s FOO ${BAR} 1\ -g FOO check_result "0 0 0 0 0 0 0 0 ${BAR}" +run_test -c -s FOO ${BAR} 1 -g FOO -c -s FOO ${NEWBAR} 1 -g FOO +check_result "0 0 ${BAR} 0 0 ${NEWBAR}" + # Unsets. run_test -u FOO -g FOO @@ -152,6 +155,10 @@ check_result "-1 22" run_test -c -s FOO ${NEWBAR} 1 -g FOO -u FOO -g FOO check_result "0 0 ${NEWBAR} 0 0" +run_test -c -u FOO -s FOO ${BAR} 1 -g FOO -u FOO -g FOO -c -u FOO\ + -s FOO ${NEWBAR} 1 -g FOO +check_result "0 0 0 0 ${BAR} 0 0 0 0 0 0 ${NEWBAR}" + # Puts. run_test -p FOO=${NEWBAR} -g FOO @@ -180,3 +187,17 @@ check_result "0 0 0 0 0 0" run_test -s FOO ${NEWBAR} 1 -p FOO=${BAR} -u FOO check_result "0 0 0 0 0 0" + +run_test -s FOO ${NEWBAR} 1 -p FOO=${BAR} -c -g FOO -p FOO=${NEWBAR} -g FOO +check_result "0 0 0 0 0 0 ${NEWBAR}" + +run_test -c -p FOO=${BAR} -g FOO -c -p FOO=${NEWBAR} -g FOO +check_result "0 0 ${BAR} 0 0 ${NEWBAR}" + + +# environ replacements. +run_test -r -g FOO -s FOO ${BAR} 1 -g FOO -u FOO -g FOO +check_result "${BAR} 0 0 ${BAR} 0 0" + +run_test -r -g FOO -u FOO -g FOO -s FOO ${BAR} 1 -g FOO +check_result "${BAR} 0 0 0 0 ${BAR}" diff --git a/tools/regression/environ/timings.c b/tools/regression/environ/timings.c index 1bf3c910a108..7999fa156d7e 100644 --- a/tools/regression/environ/timings.c +++ b/tools/regression/environ/timings.c @@ -23,7 +23,9 @@ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. */ +#include #include +#include #include #include #include @@ -64,43 +66,44 @@ int main(int argc, char **argv) { int iterations; - struct timeval endTime; - struct timeval startTime; + struct rusage endUsage; + struct rusage startUsage; /* * getenv() on the existing environment. */ - gettimeofday(&startTime, NULL); + getrusage(RUSAGE_SELF, &startUsage); /* Iterate over setting variable. */ for (iterations = 0; iterations < MaxIterations; iterations++) if (getenv(name) == NULL) err(EXIT_FAILURE, "getenv(name)"); - gettimeofday(&endTime, NULL); + getrusage(RUSAGE_SELF, &endUsage); - report_time("getenv(name)", &startTime, &endTime); + report_time("getenv(name)", &startUsage.ru_utime, &endUsage.ru_utime); /* * setenv() a variable with a large value. */ - gettimeofday(&startTime, NULL); + getrusage(RUSAGE_SELF, &startUsage); /* Iterate over setting variable. */ for (iterations = 0; iterations < MaxIterations; iterations++) if (setenv(name, value1, 1) == -1) err(EXIT_FAILURE, "setenv(name, value1, 1)"); - gettimeofday(&endTime, NULL); + getrusage(RUSAGE_SELF, &endUsage); - report_time("setenv(name, value1, 1)", &startTime, &endTime); + report_time("setenv(name, value1, 1)", &startUsage.ru_utime, + &endUsage.ru_utime); /* * getenv() the new variable on the new environment. */ - gettimeofday(&startTime, NULL); + getrusage(RUSAGE_SELF, &startUsage); /* Iterate over setting variable. */ for (iterations = 0; iterations < MaxIterations; iterations++) @@ -108,15 +111,15 @@ main(int argc, char **argv) if (getenv(name) == NULL) err(EXIT_FAILURE, "getenv(name)"); - gettimeofday(&endTime, NULL); + getrusage(RUSAGE_SELF, &endUsage); - report_time("getenv(name)", &startTime, &endTime); + report_time("getenv(name)", &startUsage.ru_utime, &endUsage.ru_utime); /* * getenv() a different variable on the new environment. */ - gettimeofday(&startTime, NULL); + getrusage(RUSAGE_SELF, &startUsage); /* Iterate over setting variable. */ for (iterations = 0; iterations < MaxIterations; iterations++) @@ -124,30 +127,31 @@ main(int argc, char **argv) if (getenv(name2) == NULL) err(EXIT_FAILURE, "getenv(name2)"); - gettimeofday(&endTime, NULL); + getrusage(RUSAGE_SELF, &endUsage); - report_time("getenv(name2)", &startTime, &endTime); + report_time("getenv(name2)", &startUsage.ru_utime, &endUsage.ru_utime); /* * setenv() a variable with a small value. */ - gettimeofday(&startTime, NULL); + getrusage(RUSAGE_SELF, &startUsage); /* Iterate over setting variable. */ for (iterations = 0; iterations < MaxIterations; iterations++) if (setenv(name, value2, 1) == -1) err(EXIT_FAILURE, "setenv(name, value2, 1)"); - gettimeofday(&endTime, NULL); + getrusage(RUSAGE_SELF, &endUsage); - report_time("setenv(name, value2, 1)", &startTime, &endTime); + report_time("setenv(name, value2, 1)", &startUsage.ru_utime, + &endUsage.ru_utime); /* * getenv() a different variable on the new environment. */ - gettimeofday(&startTime, NULL); + getrusage(RUSAGE_SELF, &startUsage); /* Iterate over setting variable. */ for (iterations = 0; iterations < MaxIterations; iterations++) @@ -155,15 +159,15 @@ main(int argc, char **argv) if (getenv(name2) == NULL) err(EXIT_FAILURE, "getenv(name)"); - gettimeofday(&endTime, NULL); + getrusage(RUSAGE_SELF, &endUsage); - report_time("getenv(name)", &startTime, &endTime); + report_time("getenv(name)", &startUsage.ru_utime, &endUsage.ru_utime); /* * getenv() a different variable on the new environment. */ - gettimeofday(&startTime, NULL); + getrusage(RUSAGE_SELF, &startUsage); /* Iterate over setting variable. */ for (iterations = 0; iterations < MaxIterations; iterations++) @@ -171,24 +175,25 @@ main(int argc, char **argv) if (getenv(name2) == NULL) err(EXIT_FAILURE, "getenv(name2)"); - gettimeofday(&endTime, NULL); + getrusage(RUSAGE_SELF, &endUsage); - report_time("getenv(name2)", &startTime, &endTime); + report_time("getenv(name2)", &startUsage.ru_utime, &endUsage.ru_utime); /* * putenv() a variable with a small value. */ - gettimeofday(&startTime, NULL); + getrusage(RUSAGE_SELF, &startUsage); /* Iterate over setting variable. */ for (iterations = 0; iterations < MaxIterations; iterations++) if (putenv(nameValuePair) == -1) err(EXIT_FAILURE, "putenv(nameValuePair)"); - gettimeofday(&endTime, NULL); + getrusage(RUSAGE_SELF, &endUsage); - report_time("putenv(nameValuePair)", &startTime, &endTime); + report_time("putenv(nameValuePair)", &startUsage.ru_utime, + &endUsage.ru_utime); exit(EXIT_SUCCESS);