From 41296effc694d12f5aa1b4dda6a7b4dbb37ce85d Mon Sep 17 00:00:00 2001 From: "Carsten Haitzler (Rasterman)" Date: Fri, 14 Aug 2020 15:22:17 +0100 Subject: [PATCH] ecore - don't do anything with heap between fork and exec 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 | 31 ++++- src/lib/ecore/efl_exe.c | 79 ++++++++---- src/lib/eina/eina_file.c | 220 +++++++++++++++++++++++++++----- 3 files changed, 271 insertions(+), 59 deletions(-) diff --git a/src/lib/ecore/ecore_exe_posix.c b/src/lib/ecore/ecore_exe_posix.c index 07f1058ab0..e72af5ae64 100644 --- a/src/lib/ecore/ecore_exe_posix.c +++ b/src/lib/ecore/ecore_exe_posix.c @@ -218,6 +218,13 @@ _impl_ecore_exe_run_priority_get(void) return run_pri; } +#if defined (__FreeBSD__) || defined (__OpenBSD__) +# include +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) { diff --git a/src/lib/ecore/efl_exe.c b/src/lib/ecore/efl_exe.c index c5a2f57343..a0fd7d4608 100644 --- a/src/lib/ecore/efl_exe.c +++ b/src/lib/ecore/efl_exe.c @@ -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! diff --git a/src/lib/eina/eina_file.c b/src/lib/eina/eina_file.c index 0d9486568f..62d3cb16c9 100644 --- a/src/lib/eina/eina_file.c +++ b/src/lib/eina/eina_file.c @@ -21,6 +21,10 @@ # include "config.h" #endif +#ifndef _GNU_SOURCE +# define _GNU_SOURCE +#endif + #include #include #include @@ -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) + 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++) { - 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 - { - 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