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.

diff --git a/src/env/getenv.c b/src/env/getenv.c
index 00c1bce03725bf51c077cf6334d826105f5ad12e..cf34672c3d60ae73d245f011a3716385c7ce3b2e 100644 (file)
--- a/src/env/getenv.c
+++ b/src/env/getenv.c
@@ -2,13 +2,14 @@
 #include <string.h>
 #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 7153042669da81a80bff37a3c25aeb1618a64da9..fa4a4ddc87ab9796127973f41b80e59d45d9f3ef 100644 (file)
--- a/src/env/putenv.c
+++ b/src/env/putenv.c
@@ -1,58 +1,48 @@
 #include <stdlib.h>
 #include <string.h>
+#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 76e8ee1206e5b5a4111b8c58481932952de6084f..a7dd2b602fc96a23181f37bf10c3729192851c94 100644 (file)
--- a/src/env/setenv.c
+++ b/src/env/setenv.c
@@ -2,29 +2,44 @@
 #include <string.h>
 #include <errno.h>
 
-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 356933546a31a41ad859c3bc08709aa76a5a2644..8630e2d760fe5d8aa1d7af0993f68ae6c137e7f1 100644 (file)
--- a/src/env/unsetenv.c
+++ b/src/env/unsetenv.c
@@ -1,31 +1,30 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
+#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;
 }