From 8e932792c917d11545c2953b35159149f7411eca Mon Sep 17 00:00:00 2001 From: Alexander Monakov Date: Sun, 3 Sep 2017 22:12:20 +0300 Subject: [PATCH] overhaul environment functions Rewrite environment access functions to slim down code, fix bugs and avoid invoking undefined behavior. * avoid using int-typed iterators where size_t would be correct; * use strncmp instead of memcmp consistently; * tighten prologues by invoking __strchrnul; * handle NULL environ. putenv: * handle "=value" input via unsetenv too (will return -1/EINVAL); * rewrite and simplify __putenv; fix the leak caused by failure to deallocate entry added by preceding setenv when called from putenv. setenv: * move management of libc-allocated entries to this translation unit, and use no-op weak symbols in putenv/unsetenv; unsetenv: * rewrite; this fixes UB caused by testing a free'd pointer against NULL on entry to subsequent loops. Not changed: Failure to extend allocation tracking array (previously __env_map, now env_alloced) is ignored rather than causing to report -1/ENOMEM to the caller; the worst-case consequence is leaking this allocation when it is removed or replaced in a subsequent environment access. Initially UB in unsetenv was reported by Alexander Cherepanov. Using a weak alias to avoid pulling in malloc via unsetenv was suggested by Rich Felker. --- src/env/getenv.c | 15 ++++----- src/env/putenv.c | 76 ++++++++++++++++++++-------------------------- src/env/setenv.c | 41 +++++++++++++++++-------- src/env/unsetenv.c | 35 +++++++++++---------- 4 files changed, 86 insertions(+), 81 deletions(-) diff --git a/src/env/getenv.c b/src/env/getenv.c index 00c1bce0..cf34672c 100644 --- a/src/env/getenv.c +++ b/src/env/getenv.c @@ -2,13 +2,14 @@ #include #include "libc.h" +char *__strchrnul(const char *, int); + char *getenv(const char *name) { - int i; - size_t l = strlen(name); - if (!__environ || !*name || strchr(name, '=')) return NULL; - for (i=0; __environ[i] && (strncmp(name, __environ[i], l) - || __environ[i][l] != '='); i++); - if (__environ[i]) return __environ[i] + l+1; - return NULL; + size_t l = __strchrnul(name, '=') - name; + if (l && !name[l] && __environ) + for (char **e = __environ; *e; e++) + if (!strncmp(name, *e, l) && l[*e] == '=') + return *e + l+1; + return 0; } diff --git a/src/env/putenv.c b/src/env/putenv.c index 71530426..fa4a4ddc 100644 --- a/src/env/putenv.c +++ b/src/env/putenv.c @@ -1,58 +1,48 @@ #include #include +#include "libc.h" -extern char **__environ; -char **__env_map; +char *__strchrnul(const char *, int); -int __putenv(char *s, int a) -{ - int i=0, j=0; - char *z = strchr(s, '='); - char **newenv = 0; - char **newmap = 0; - static char **oldenv; +static void dummy(char *old, char *new) {} +weak_alias(dummy, __env_rm_add); - if (!z) return unsetenv(s); - if (z==s) return -1; - for (; __environ[i] && memcmp(s, __environ[i], z-s+1); i++); - if (a) { - if (!__env_map) { - __env_map = calloc(2, sizeof(char *)); - if (__env_map) __env_map[0] = s; - } else { - for (; __env_map[j] && __env_map[j] != __environ[i]; j++); - if (!__env_map[j]) { - newmap = realloc(__env_map, sizeof(char *)*(j+2)); - if (newmap) { - __env_map = newmap; - __env_map[j] = s; - __env_map[j+1] = NULL; - } - } else { - free(__env_map[j]); - __env_map[j] = s; +int __putenv(char *s, size_t l, char *r) +{ + size_t i=0; + if (__environ) { + for (char **e = __environ; *e; e++, i++) + if (!strncmp(s, *e, l+1)) { + char *tmp = *e; + *e = s; + __env_rm_add(tmp, r); + return 0; } - } } - if (!__environ[i]) { - newenv = malloc(sizeof(char *)*(i+2)); - if (!newenv) { - if (a && __env_map) __env_map[j] = 0; - return -1; - } - memcpy(newenv, __environ, sizeof(char *)*i); - newenv[i] = s; - newenv[i+1] = 0; - __environ = newenv; + static char **oldenv; + char **newenv; + if (__environ == oldenv) { + newenv = realloc(oldenv, sizeof *newenv * (i+2)); + if (!newenv) goto oom; + } else { + newenv = malloc(sizeof *newenv * (i+2)); + if (!newenv) goto oom; + if (i) memcpy(newenv, __environ, sizeof *newenv * i); free(oldenv); - oldenv = __environ; } - - __environ[i] = s; + newenv[i] = s; + newenv[i+1] = 0; + __environ = oldenv = newenv; + if (r) __env_rm_add(0, r); return 0; +oom: + free(r); + return -1; } int putenv(char *s) { - return __putenv(s, 0); + size_t l = __strchrnul(s, '=') - s; + if (!l || !s[l]) return unsetenv(s); + return __putenv(s, l, 0); } diff --git a/src/env/setenv.c b/src/env/setenv.c index 76e8ee12..a7dd2b60 100644 --- a/src/env/setenv.c +++ b/src/env/setenv.c @@ -2,29 +2,44 @@ #include #include -int __putenv(char *s, int a); +char *__strchrnul(const char *, int); +int __putenv(char *, size_t, char *); + +void __env_rm_add(char *old, char *new) +{ + static char **env_alloced; + static size_t env_alloced_n; + for (size_t i=0; i < env_alloced_n; i++) + if (env_alloced[i] == old) { + env_alloced[i] = new; + free(old); + return; + } else if (!env_alloced[i] && new) { + env_alloced[i] = new; + new = 0; + } + if (!new) return; + char **t = realloc(env_alloced, sizeof *t * (env_alloced_n+1)); + if (!t) return; + (env_alloced = t)[env_alloced_n++] = new; +} int setenv(const char *var, const char *value, int overwrite) { char *s; - int l1, l2; + size_t l1, l2; - if (!var || !*var || strchr(var, '=')) { + if (!var || !(l1 = __strchrnul(var, '=') - var) || var[l1]) { errno = EINVAL; return -1; } if (!overwrite && getenv(var)) return 0; - l1 = strlen(var); l2 = strlen(value); s = malloc(l1+l2+2); - if (s) { - memcpy(s, var, l1); - s[l1] = '='; - memcpy(s+l1+1, value, l2); - s[l1+l2+1] = 0; - if (!__putenv(s, 1)) return 0; - } - free(s); - return -1; + if (!s) return -1; + memcpy(s, var, l1); + s[l1] = '='; + memcpy(s+l1+1, value, l2+1); + return __putenv(s, l1, s); } diff --git a/src/env/unsetenv.c b/src/env/unsetenv.c index 35693354..8630e2d7 100644 --- a/src/env/unsetenv.c +++ b/src/env/unsetenv.c @@ -1,31 +1,30 @@ #include #include #include +#include "libc.h" -extern char **__environ; -extern char **__env_map; +char *__strchrnul(const char *, int); + +static void dummy(char *old, char *new) {} +weak_alias(dummy, __env_rm_add); int unsetenv(const char *name) { - int i, j; - size_t l = strlen(name); - - if (!*name || strchr(name, '=')) { + size_t l = __strchrnul(name, '=') - name; + if (!l || name[l]) { errno = EINVAL; return -1; } -again: - for (i=0; __environ[i] && (memcmp(name, __environ[i], l) || __environ[i][l] != '='); i++); - if (__environ[i]) { - if (__env_map) { - for (j=0; __env_map[j] && __env_map[j] != __environ[i]; j++); - free (__env_map[j]); - for (; __env_map[j]; j++) - __env_map[j] = __env_map[j+1]; - } - for (; __environ[i]; i++) - __environ[i] = __environ[i+1]; - goto again; + if (__environ) { + char **e = __environ, **eo = e; + for (; *e; e++) + if (!strncmp(name, *e, l) && l[*e] == '=') + __env_rm_add(*e, 0); + else if (eo != e) + *eo++ = *e; + else + eo++; + if (eo != e) *eo = 0; } return 0; } -- 2.25.1