ecore - don't do anything with heap between fork and exec
authorCarsten Haitzler (Rasterman) <raster@rasterman.com>
Fri, 14 Aug 2020 14:22:17 +0000 (15:22 +0100)
committerJongmin Lee <jm105.lee@samsung.com>
Thu, 20 Aug 2020 21:03:30 +0000 (06:03 +0900)
this avoids a possibgle deadlock if a malloc impl is holding a lock
and has not released it at the time we fork.

@fix

src/lib/ecore/ecore_exe_posix.c
src/lib/ecore/efl_exe.c
src/lib/eina/eina_file.c

index f88f0ca..895875c 100644 (file)
@@ -218,6 +218,13 @@ _impl_ecore_exe_run_priority_get(void)
    return run_pri;
 }
 
+#if defined (__FreeBSD__) || defined (__OpenBSD__)
+# include <dlfcn.h>
+static char ***_dl_environ;
+#else
+extern char **environ;
+#endif
+
 Eo *
 _impl_ecore_exe_efl_object_finalize(Eo *obj, Ecore_Exe_Data *exe)
 {
@@ -294,7 +301,29 @@ _impl_ecore_exe_efl_object_finalize(Eo *obj, Ecore_Exe_Data *exe)
       else if (pid == 0) /* child */
       {
 #ifdef HAVE_SYSTEMD
-         unsetenv("NOTIFY_SOCKET");
+         char **env = NULL, **e;
+
+# if defined (__FreeBSD__) || defined (__OpenBSD__)
+         _dl_environ = dlsym(NULL, "environ");
+         env = *_dl_environ;
+# else
+         env = environ;
+# endif
+         // find NOTIFY_SOCKET env var and remove it without any heap work
+         if (env)
+           {
+              Eina_Bool shuffle = EINA_FALSE;
+
+              for (e = env; *e; e++)
+                {
+                   if (!shuffle)
+                     {
+                        if (!strncmp(e[0], "NOTIFY_SOCKET=", 14))
+                          shuffle = EINA_TRUE;
+                     }
+                   if (shuffle) e[0] = e[1];
+                }
+           }
 #endif
          if (run_pri != ECORE_EXE_PRIORITY_INHERIT)
          {
index c5a2f57..a0fd7d4 100644 (file)
@@ -357,8 +357,10 @@ _efl_exe_efl_task_run(Eo *obj, Efl_Exe_Data *pd)
    return EINA_FALSE;
 #else
    Eo *loop;
-   Efl_Task_Data *tdl, *td = efl_data_scope_get(obj, EFL_TASK_CLASS);
+   Efl_Task_Data *tdl = NULL, *td = efl_data_scope_get(obj, EFL_TASK_CLASS);
+   Eina_Iterator *itr = NULL, *itr2 = NULL;
    const char *cmd;
+   char **newenv, **env = NULL, **e;
    int devnull;
    int pipe_stdin[2];
    int pipe_stdout[2];
@@ -428,9 +430,17 @@ _efl_exe_efl_task_run(Eo *obj, Efl_Exe_Data *pd)
      }
 
    _ecore_signal_pid_lock();
+   // get these before the fork to avoid heap malloc deadlocks
+   loop = efl_provider_find(obj, EFL_LOOP_CLASS);
+   if (loop) tdl = efl_data_scope_get(loop, EFL_TASK_CLASS);
+   if (pd->env) itr = efl_core_env_content_get(pd->env);
+   if (pd->env) itr2 = efl_core_env_content_get(pd->env);
    pd->pid = fork();
+
    if (pd->pid != 0)
      {
+        if (itr) eina_iterator_free(itr);
+        if (itr2) eina_iterator_free(itr2);
         // parent process is here inside this if block
         if (td->flags & EFL_TASK_FLAGS_USE_STDIN)  close(pipe_stdin[0]);
         if (td->flags & EFL_TASK_FLAGS_USE_STDOUT) close(pipe_stdout[1]);
@@ -513,58 +523,77 @@ _efl_exe_efl_task_run(Eo *obj, Efl_Exe_Data *pd)
         close(devnull);
      }
 
-   if (!(loop = efl_provider_find(obj, EFL_LOOP_CLASS))) exit(1);
-
-   if (!(tdl = efl_data_scope_get(loop, EFL_TASK_CLASS))) exit(1);
+   if (!tdl) exit(1);
 
    // clear systemd notify socket... only relevant for systemd world,
    // otherwise shouldn't be trouble
-   putenv("NOTIFY_SOCKET=");
+# if defined (__FreeBSD__) || defined (__OpenBSD__)
+   _dl_environ = dlsym(NULL, "environ");
+   if (_dl_environ) env = *_dl_environ;
+# else
+   env = environ;
+# endif
+   if (env)
+     {
+        Eina_Bool shuffle = EINA_FALSE;
+
+        for (e = env; *e; e++)
+          {
+             if (!shuffle)
+               {
+                  if (!strncmp(e[0], "NOTIFY_SOCKET=", 14))
+                    shuffle = EINA_TRUE;
+               }
+             if (shuffle) e[0] = e[1];
+          }
+     }
 
    // actually setenv the env object (clear what was there before so it is
    // the only env there)
    if (pd->env)
      {
-        Eina_Iterator *itr;
         const char *key;
+        int count = 0, i = 0;
 
-# ifdef HAVE_CLEARENV
-        clearenv();
-# else
-#  if defined (__FreeBSD__) || defined (__OpenBSD__)
-        _dl_environ = dlsym(NULL, "environ");
-        if (_dl_environ) *_dl_environ = NULL;
-        else ERR("Can't find envrion symbol");
-#  else
-        environ = NULL;
-#  endif
-# endif
-        itr = efl_core_env_content_get(pd->env);
-
+        // use 2nd throw-away itr to count
+        EINA_ITERATOR_FOREACH(itr2, key)
+          {
+             count++;
+          }
+        // object which we don't free (sitting in hash table in env obj)
+        newenv = alloca(sizeof(char *) * (count + 1));
+        // use 2st iter to walk and fill new env
         EINA_ITERATOR_FOREACH(itr, key)
           {
-             setenv(key, efl_core_env_get(pd->env, key) , 1);
+             newenv[i] = (char *)efl_core_env_get(pd->env, key);
+             i++;
           }
-       efl_unref(pd->env);
-       pd->env = NULL;
+        // yes - we dont free itr or itr2 - we're going to exec below or exit
+        // also put newenv array on stack pointign to the strings in the env
+# if defined (__FreeBSD__) || defined (__OpenBSD__)
+        if (_dl_environ) *_dl_environ = newenv;
+        else ERR("Can't find envrion symbol");
+# else
+        environ = newenv;
+# endif
      }
 
    // close all fd's other than the first 3 (0, 1, 2) and exited write fd
    int except[2] = { 0, -1 };
    except[0] = pd->fd.exited_write;
    eina_file_close_from(3, except);
-#ifdef HAVE_PRCTL
+# ifdef HAVE_PRCTL
    if ((pd->flags & EFL_EXE_FLAGS_TERM_WITH_PARENT))
      {
         prctl(PR_SET_PDEATHSIG, SIGTERM);
      }
-#elif defined(HAVE_PROCCTL)
+# elif defined(HAVE_PROCCTL)
    if ((pd->flags & EFL_EXE_FLAGS_TERM_WITH_PARENT))
      {
         int sig = SIGTERM;
         procctl(P_PID, 0, PROC_PDEATHSIG_CTL, &sig);
      }
-#endif
+# endif
    // actually execute!
    _exec(cmd, pd->flags, td->flags);
    // we couldn't exec... uh oh. HAAAAAAAALP!
index c8abe6d..e38bf5b 100644 (file)
 # include "config.h"
 #endif
 
+#ifndef _GNU_SOURCE
+# define _GNU_SOURCE
+#endif
+
 #include <stdlib.h>
 #include <string.h>
 #include <stddef.h>
@@ -1252,6 +1256,47 @@ eina_file_statat(void *container, Eina_File_Direct_Info *info, Eina_Stat *st)
    return 0;
 }
 
+///////////////////////////////////////////////////////////////////////////
+// this below is funky avoiding opendir to avoid heap allocations thus
+// getdents and all the os specific stuff as this is intendedf for use
+// between fork and exec normally ... this is important
+#if defined(__FreeBSD__)
+# define do_getdents(fd, buf, size) getdents(fd, buf, size)
+typedef struct
+{
+   ino_t          d_ino;
+   off_t          d_off;
+   unsigned short d_reclen;
+   unsigned char  d_type;
+   unsigned char  ____pad0;
+   unsigned short d_namlen;
+   unsigned short ____pad1;
+   char           d_name[4096];
+} Dirent;
+#elif defined(__OpenBSD__)
+# define do_getdents(fd, buf, size) getdents(fd, buf, size)
+typedef struct
+{
+   __ino_t        d_ino;
+   __off_t        d_off;
+   unsigned short d_reclen;
+   unsigned char  d_type;
+   unsigned char  d_namlen;
+   unsigned char  ____pad[4];
+   char           d_name[4096];
+} Dirent;
+#elif defined(__linux__)
+# define do_getdents(fd, buf, size) getdents64(fd, buf, size)
+typedef struct
+{
+   ino64_t        d_ino;
+   off64_t        d_off;
+   unsigned short d_reclen;
+   unsigned char  d_type;
+   char           d_name[4096];
+} Dirent;
+#endif
+
 EAPI void
 eina_file_close_from(int fd, int *except_fd)
 {
@@ -1259,62 +1304,171 @@ eina_file_close_from(int fd, int *except_fd)
    // XXX: what do to here? anything?
 #else
 #ifdef HAVE_DIRENT_H
+//# if 0
+# if defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__linux__)
+   int dirfd;
+   Dirent *d;
+   char buf[4096];
+   int *closes = NULL;
+   int num_closes = 0, i, j, clo, num;
+   const char *fname;
+   ssize_t pos, ret;
+   Eina_Bool do_read;
+
+   dirfd = open("/proc/self/fd", O_RDONLY | O_DIRECTORY);
+   if (dirfd < 0) dirfd = open("/dev/fd", O_RDONLY | O_DIRECTORY);
+   if (dirfd >= 0)
+     {
+        // count # of closes - the dir list should/will not change as its
+        // the fd's we have open so we can read it twice with no changes
+        // to it
+        do_read = EINA_TRUE;
+        for (;;)
+          {
+skip:
+             if (do_read)
+               {
+                  pos = 0;
+                  ret = do_getdents(dirfd, buf, sizeof(buf));
+                  if (ret <= 0) break;
+                  do_read = EINA_FALSE;
+               }
+             d = (Dirent *)(buf + pos);
+             fname = d->d_name;
+             pos += d->d_reclen;
+             if (pos >= ret) do_read = EINA_TRUE;
+             if (!((fname[0] >= '0') && (fname[0] <= '9'))) continue;
+             num = atoi(fname);
+             if (num < fd) continue;
+             if (except_fd)
+               {
+                  for (j = 0; except_fd[j] >= 0; j++)
+                    {
+                       if (except_fd[j] == num) goto skip;
+                    }
+               }
+             num_closes++;
+          }
+        // alloc closes list and walk again to fill it - on stack to avoid
+        // heap allocs
+        closes = alloca(num_closes * sizeof(int));
+        if ((closes) && (num_closes > 0))
+          {
+             clo = 0;
+             lseek(dirfd, 0, SEEK_SET);
+             do_read = EINA_TRUE;
+             for (;;)
+               {
+skip2:
+                  if (do_read)
+                    {
+                       pos = 0;
+                       ret = do_getdents(dirfd, buf, sizeof(buf));
+                       if (ret <= 0) break;
+                       do_read = EINA_FALSE;
+                    }
+                  d = (Dirent *)(buf + pos);
+                  fname = d->d_name;
+                  pos += d->d_reclen;
+                  if (pos >= ret) do_read = EINA_TRUE;
+                  if (!((fname[0] >= '0') && (fname[0] <= '9'))) continue;
+                  num = atoi(fname);
+                  if (num < fd) continue;
+                  if (except_fd)
+                    {
+                       for (j = 0; except_fd[j] >= 0; j++)
+                         {
+                            if (except_fd[j] == num) goto skip2;
+                         }
+                    }
+                  if (clo < num_closes) closes[clo] = num;
+                  clo++;
+               }
+          }
+        close(dirfd);
+        // now go close all those fd's - some may be invalide like the dir
+        // reading fd above... that's ok.
+        for (i = 0; i < num_closes; i++)
+          {
+             close(closes[i]);
+          }
+        return;
+     }
+# else
    DIR *dir;
+   int *closes = NULL;
+   int num_closes = 0, i, j, clo, num;
+   struct dirent *dp;
+   const char *fname;
 
    dir = opendir("/proc/self/fd");
    if (!dir) dir = opendir("/dev/fd");
    if (dir)
      {
-        struct dirent *dp;
-        const char *fname;
-        int *closes = NULL;
-        int num_closes = 0, i;
-
+        // count # of closes - the dir list should/will not change as its
+        // the fd's we have open so we can read it twice with no changes
+        // to it
         for (;;)
           {
 skip:
-             dp = readdir(dir);
-             if (!dp) break;
+             if (!(dp = readdir(dir))) break;
              fname = dp->d_name;
-
-             if ((fname[0] >= '0') && (fname[0] <= '9'))
+             if (!((fname[0] >= '0') && (fname[0] <= '9'))) continue;
+             num = atoi(fname);
+             if (num < fd) continue;
+             if (except_fd)
                {
-                  int num = atoi(fname);
-                  if (num >= fd)
+                  for (j = 0; except_fd[j] >= 0; j++)
                     {
-                       if (except_fd)
-                         {
-                            int j;
-
-                            for (j = 0; except_fd[j] >= 0; j++)
-                              {
-                                 if (except_fd[j] == num) goto skip;
-                              }
-                         }
-                       num_closes++;
-                       int *tmp = realloc(closes, num_closes * sizeof(int));
-                       if (!tmp) num_closes--;
-                       else
+                       if (except_fd[j] == num) goto skip;
+                    }
+               }
+             num_closes++;
+          }
+        // alloc closes list and walk again to fill it - on stack to avoid
+        // heap allocs
+        closes = alloca(num_closes * sizeof(int));
+        if ((closes) && (num_closes > 0))
+          {
+             clo = 0;
+             seekdir(dir, 0);
+             for (;;)
+               {
+skip2:
+                  if (!(dp = readdir(dir))) break;
+                  fname = dp->d_name;
+                  if (!((fname[0] >= '0') && (fname[0] <= '9'))) continue;
+                  num = atoi(fname);
+                  if (num < fd) continue;
+                  if (except_fd)
+                    {
+                       for (j = 0; except_fd[j] >= 0; j++)
                          {
-                            closes = tmp;
-                            closes[num_closes - 1] = num;
+                            if (except_fd[j] == num) goto skip2;
                          }
                     }
+                  if (clo < num_closes) closes[clo] = num;
+                  clo++;
                }
           }
         closedir(dir);
-        for (i = 0; i < num_closes; i++) close(closes[i]);
-        free(closes);
+        // now go close all those fd's - some may be invalide like the dir
+        // reading fd above... that's ok.
+        for (i = 0; i < num_closes; i++)
+          {
+             close(closes[i]);
+          }
         return;
      }
+# endif
 #endif
-   int i, max = 1024;
+   int max = 1024;
 
-# ifdef HAVE_SYS_RESOURCE_H
+#ifdef HAVE_SYS_RESOURCE_H
    struct rlimit lim;
    if (getrlimit(RLIMIT_NOFILE, &lim) < 0) return;
    max = lim.rlim_max;
-# endif
+#endif
    for (i = fd; i < max;)
      {
         if (except_fd)
@@ -1323,11 +1477,11 @@ skip:
 
              for (j = 0; except_fd[j] >= 0; j++)
                {
-                  if (except_fd[j] == i) goto skip2;
+                  if (except_fd[j] == i) goto skip3;
                }
           }
         close(i);
-skip2:
+skip3:
         i++;
      }
 #endif