From c6a8697800376a02b868cdea8fc1bf55f12798f1 Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Mon, 16 Dec 2019 23:14:45 +0100 Subject: Introduce helper functions for safe fork+exec of processes In some situations, we want to execute an external shell command in a non-blocking way. Similar to 'system', but without waiting for the child to complete. We also want to close all file descriptors ahead of the exec() and filter + modify the environment. Change-Id: Ib24ac8a083db32e55402ce496a5eabd8749cc888 Related: OS#4332 --- configure.ac | 4 +- include/Makefile.am | 1 + include/osmocom/core/exec.h | 28 ++++++ src/Makefile.am | 1 + src/exec.c | 238 ++++++++++++++++++++++++++++++++++++++++++++ tests/Makefile.am | 10 +- tests/exec/exec_test.c | 155 +++++++++++++++++++++++++++++ tests/exec/exec_test.err | 1 + tests/exec/exec_test.ok | 46 +++++++++ tests/testsuite.at | 7 ++ 10 files changed, 487 insertions(+), 4 deletions(-) create mode 100644 include/osmocom/core/exec.h create mode 100644 src/exec.c create mode 100644 tests/exec/exec_test.c create mode 100644 tests/exec/exec_test.err create mode 100644 tests/exec/exec_test.ok diff --git a/configure.ac b/configure.ac index e45ec9f1..1056a0a2 100644 --- a/configure.ac +++ b/configure.ac @@ -282,7 +282,7 @@ AC_ARG_ENABLE(embedded, )], [embedded=$enableval], [embedded="no"]) -AM_CONDITIONAL(ENABLE_STATS_TEST, true) +AM_CONDITIONAL(EMBEDDED, false) AM_CONDITIONAL(ENABLE_SERCOM_STUB, false) if test x"$embedded" = x"yes" @@ -301,7 +301,7 @@ then AM_CONDITIONAL(ENABLE_PCSC, false) AM_CONDITIONAL(ENABLE_PSEUDOTALLOC, true) AM_CONDITIONAL(ENABLE_SERCOM_STUB, true) - AM_CONDITIONAL(ENABLE_STATS_TEST, false) + AM_CONDITIONAL(EMBEDDED, true) AC_DEFINE([USE_GNUTLS], [0]) AC_DEFINE([PANIC_INFLOOP],[1],[Use infinite loop on panic rather than fprintf/abort]) fi diff --git a/include/Makefile.am b/include/Makefile.am index dc6eaa7f..b341ee3c 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -23,6 +23,7 @@ nobase_include_HEADERS = \ osmocom/core/crcgen.h \ osmocom/core/endian.h \ osmocom/core/defs.h \ + osmocom/core/exec.h \ osmocom/core/fsm.h \ osmocom/core/gsmtap.h \ osmocom/core/gsmtap_util.h \ diff --git a/include/osmocom/core/exec.h b/include/osmocom/core/exec.h new file mode 100644 index 00000000..6bbd352c --- /dev/null +++ b/include/osmocom/core/exec.h @@ -0,0 +1,28 @@ +#pragma once +/* (C) 2019 by Harald Welte + * + * All Rights Reserved + * + * SPDX-License-Identifier: GPL-2.0+ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ +extern const char *osmo_environment_whitelist[]; + +int osmo_environment_filter(char **out, size_t out_len, char **in, const char **whitelist); +int osmo_environment_append(char **out, size_t out_len, char **in); +int osmo_close_all_fds_above(int last_fd_to_keep); +int osmo_system_nowait(const char *command, const char **env_whitelist, char **addl_env); diff --git a/src/Makefile.am b/src/Makefile.am index 99432811..eeb3f7df 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -27,6 +27,7 @@ libosmocore_la_SOURCES = context.c timer.c timer_gettimeofday.c timer_clockgetti tdef.c \ sockaddr_str.c \ use_count.c \ + exec.c \ $(NULL) if HAVE_SSSE3 diff --git a/src/exec.c b/src/exec.c new file mode 100644 index 00000000..a9d8ce0f --- /dev/null +++ b/src/exec.c @@ -0,0 +1,238 @@ +/* (C) 2019 by Harald Welte + * + * All Rights Reserved + * + * SPDX-License-Identifier: GPL-2.0+ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ + +#include "config.h" +#ifndef EMBEDDED + +#include + +#include +#include + +#include +#include +#include + +#include +#include +#include + +/*! suggested list of environment variables to pass (if they exist) to a sub-process/script */ +const char *osmo_environment_whitelist[] = { + "USER", "LOGNAME", "HOME", + "LANG", "LC_ALL", "LC_COLLATE", "LC_CTYPE", "LC_MESSAGES", "LC_MONETARY", "LC_NUMERIC", "LC_TIME", + "PATH", + "PWD", + "SHELL", + "TERM", + "TMPDIR", + "LD_LIBRARY_PATH", + "LD_PRELOAD", + "POSIXLY_CORRECT", + "HOSTALIASES", + "TZ", "TZDIR", + "TERMCAP", + "COLUMNS", "LINES", + NULL +}; + +static bool str_in_list(const char **list, const char *key) +{ + const char **ent; + + for (ent = list; *ent; ent++) { + if (!strcmp(*ent, key)) + return true; + } + return false; +} + +/*! filtered a process environment by whitelist; only copying pointers, no actual strings. + * + * This function is useful if you'd like to generate an environment to pass exec*e() + * functions. It will create a new environment containing only those entries whose + * keys (as per environment convention KEY=VALUE) are contained in the whitelist. The + * function will not copy the actual strings, but just create a new pointer array, pointing + * to the same memory as the input strings. + * + * Constraints: Keys up to a maximum length of 255 characters are supported. + * + * \oaram[out] out caller-allocated array of pointers for the generated output + * \param[in] out_len size of out (number of pointers) + * \param[in] in input environment (NULL-terminated list of pointers like **environ) + * \param[in] whitelist whitelist of permitted keys in environment (like **environ) + * \returns number of entries filled in 'out'; negtive on error */ +int osmo_environment_filter(char **out, size_t out_len, char **in, const char **whitelist) +{ + char tmp[256]; + char **ent; + size_t out_used = 0; + + /* invalid calls */ + if (!out || out_len == 0 || !whitelist) + return -EINVAL; + + /* legal, but unusual: no input to filter should generate empty, terminated out */ + if (!in) { + out[0] = NULL; + return 1; + } + + /* iterate over input entries */ + for (ent = in; *ent; ent++) { + char *eq = strchr(*ent, '='); + unsigned long eq_pos; + if (!eq) { + /* no '=' in string, skip it */ + continue; + } + eq_pos = eq - *ent; + if (eq_pos >= ARRAY_SIZE(tmp)) + continue; + strncpy(tmp, *ent, eq_pos); + tmp[eq_pos] = '\0'; + if (str_in_list(whitelist, tmp)) { + if (out_used == out_len-1) + break; + /* append to output */ + out[out_used++] = *ent; + } + } + OSMO_ASSERT(out_used < out_len); + out[out_used++] = NULL; + return out_used; +} + +/*! append one environment to another; only copying pointers, not actual strings. + * + * This function is useful if you'd like to append soem entries to an environment + * befoer passing it to exec*e() functions. + * + * It will append all entries from 'in' to the environment in 'out', as long as + * 'out' has space (determined by 'out_len'). + * + * Constraints: If the same key exists in 'out' and 'in', duplicate keys are + * generated. It is a simple append, without any duplicate checks. + * + * \oaram[out] out caller-allocated array of pointers for the generated output + * \param[in] out_len size of out (number of pointers) + * \param[in] in input environment (NULL-terminated list of pointers like **environ) + * \returns number of entries filled in 'out'; negative on error */ +int osmo_environment_append(char **out, size_t out_len, char **in) +{ + size_t out_used = 0; + + if (!out || out_len == 0) + return -EINVAL; + + /* seek to end of existing output */ + for (out_used = 0; out[out_used]; out_used++) {} + + if (!in) { + if (out_used == 0) + out[out_used++] = NULL; + return out_used; + } + + for (; *in && out_used < out_len-1; in++) + out[out_used++] = *in; + + OSMO_ASSERT(out_used < out_len); + out[out_used++] = NULL; + + return out_used; +} + +/* Iterate over files in /proc/self/fd and close all above lst_fd_to_keep */ +int osmo_close_all_fds_above(int last_fd_to_keep) +{ + struct dirent *ent; + DIR *dir; + int rc; + + dir = opendir("/proc/self/fd"); + if (!dir) { + LOGP(DLGLOBAL, LOGL_ERROR, "Cannot open /proc/self/fd: %s\n", strerror(errno)); + return -ENODEV; + } + + while ((ent = readdir(dir))) { + int fd = atoi(ent->d_name); + if (fd <= last_fd_to_keep) + continue; + if (fd == dirfd(dir)) + continue; + rc = close(fd); + if (rc) + LOGP(DLGLOBAL, LOGL_ERROR, "Error closing fd=%d: %s\n", fd, strerror(errno)); + } + closedir(dir); + return 0; +} + +/* Seems like POSIX has no header file for this, and even glibc + __USE_GNU doesn't help */ +extern char **environ; + +/*! call an external shell command without waiting for it. + * + * This mimics the behavior of system(3), with the following differences: + * - it doesn't wait for completion of the child process + * - it closes all non-stdio file descriptors by iterating /proc/self/fd + * - it constructs a reduced environment where only whitelisted keys survive + * - it (optionally) appends additional variables to the environment + * + * \param[in] command the shell command to be executed, see system(3) + * \param[in] env_whitelist A white-list of keys for environment variables + * \param[in] addl_env any additional environment variables to be appended + * \returns PID of generated child process; negative on error + */ +int osmo_system_nowait(const char *command, const char **env_whitelist, char **addl_env) +{ + int rc; + + rc = fork(); + if (rc == 0) { + /* we are in the child */ + char *new_env[1024]; + + /* close all file descriptors above stdio */ + osmo_close_all_fds_above(2); + + /* build the new environment */ + if (env_whitelist) + osmo_environment_filter(new_env, ARRAY_SIZE(new_env), environ, env_whitelist); + if (addl_env) + osmo_environment_append(new_env, ARRAY_SIZE(new_env), addl_env); + + /* if we want to behave like system(3), we must go via the shell */ + execle("/bin/sh", "sh", "-c", command, (char *) NULL, new_env); + /* only reached in case of error */ + LOGP(DLGLOBAL, LOGL_ERROR, "Error executing command '%s' after fork: %s\n", + command, strerror(errno)); + return -EIO; + } else { + /* we are in the parent */ + return rc; + } +} + +#endif /* EMBEDDED */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 3a3ea376..bf7017b1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -60,8 +60,10 @@ check_PROGRAMS += \ $(NULL) endif -if ENABLE_STATS_TEST -check_PROGRAMS += stats/stats_test +if !EMBEDDED +check_PROGRAMS += \ + stats/stats_test \ + exec/exec_test endif if ENABLE_GB @@ -259,6 +261,9 @@ use_count_use_count_test_LDADD = $(LDADD) context_context_test_SOURCES = context/context_test.c context_context_test_LDADD = $(LDADD) +exec_exec_test_SOURCES = exec/exec_test.c +exec_exec_test_LDADD = $(LDADD) + # The `:;' works around a Bash 3.2 bug when the output is not writeable. $(srcdir)/package.m4: $(top_srcdir)/configure.ac :;{ \ @@ -334,6 +339,7 @@ EXTRA_DIST = testsuite.at $(srcdir)/package.m4 $(TESTSUITE) \ use_count/use_count_test.ok use_count/use_count_test.err \ context/context_test.ok \ gsm0502/gsm0502_test.ok \ + exec/exec_test.ok exec/exec_test.err \ $(NULL) DISTCLEANFILES = atconfig atlocal conv/gsm0503_test_vectors.c diff --git a/tests/exec/exec_test.c b/tests/exec/exec_test.c new file mode 100644 index 00000000..5f4b460c --- /dev/null +++ b/tests/exec/exec_test.c @@ -0,0 +1,155 @@ +#include +#include + +#include +#include +#include +#include +#include +#include + +static void env_dump(char **env) +{ + char **ent; + + for (ent = env; *ent; ent++) + printf("\t%s\n", *ent); +} + +static void test_env_filter(void) +{ + char *out[256]; + char *env_in[] = { + "FOO=1", + "BAR=2", + "USER=mahlzeit", + "BAZ=3", + "SHELL=/bin/sh", + NULL + }; + const char *filter[] = { + "SHELL", + "USER", + NULL + }; + int rc; + + printf("\n==== osmo_environment_filter ====\n"); + + printf("Input Environment:\n"); + env_dump(env_in); + printf("Input Whitelist:\n"); + env_dump((char **) filter); + rc = osmo_environment_filter(out, ARRAY_SIZE(out), env_in, filter); + printf("Output Environment (%d):\n", rc); + env_dump(out); + OSMO_ASSERT(rc == 3); + + printf("Testing for NULL out\n"); + rc = osmo_environment_filter(NULL, 123, env_in, filter); + OSMO_ASSERT(rc < 0); + + printf("Testing for zero-length out\n"); + rc = osmo_environment_filter(out, 0, env_in, filter); + OSMO_ASSERT(rc < 0); + + printf("Testing for one-length out\n"); + rc = osmo_environment_filter(out, 1, env_in, filter); + OSMO_ASSERT(rc == 1 && out[0] == NULL); + + printf("Testing for no filter\n"); + rc = osmo_environment_filter(out, ARRAY_SIZE(out), env_in, NULL); + OSMO_ASSERT(rc < 0); + + printf("Testing for no input\n"); + rc = osmo_environment_filter(out, ARRAY_SIZE(out), NULL, filter); + OSMO_ASSERT(rc == 1 && out[0] == NULL); + printf("Success!\n"); +} + +static void test_env_append(void) +{ + char *out[256] = { + "FOO=a", + "BAR=b", + "BAZ=c", + NULL, + }; + char *add[] = { + "MAHL=zeit", + "GSM=global", + "UMTS=universal", + "LTE=evolved", + NULL, + }; + int rc; + + printf("\n==== osmo_environment_append ====\n"); + + printf("Input Environment:\n"); + env_dump(out); + printf("Input Addition:\n"); + env_dump(add); + rc = osmo_environment_append(out, ARRAY_SIZE(out), add); + printf("Output Environment (%d)\n", rc); + env_dump(out); + OSMO_ASSERT(rc == 8); + printf("Success!\n"); +} + +static void test_close_fd(void) +{ + struct stat st; + int fds[2]; + int rc; + + printf("\n==== osmo_close_all_fds_above ====\n"); + + /* create some extra fds */ + rc = socketpair(AF_UNIX, SOCK_STREAM, 0, fds); + OSMO_ASSERT(rc == 0); + + rc = fstat(fds[0], &st); + OSMO_ASSERT(rc == 0); + + osmo_close_all_fds_above(2); + + rc = fstat(fds[0], &st); + OSMO_ASSERT(rc == -1 && errno == EBADF); + rc = fstat(fds[1], &st); + OSMO_ASSERT(rc == -1 && errno == EBADF); + printf("Success!\n"); +} + +static void test_system_nowait(void) +{ + char *addl_env[] = { + "MAHLZEIT=spaet", + NULL + }; + int rc, pid, i; + + printf("\n==== osmo_system_nowait ====\n"); + + pid = osmo_system_nowait("env | grep MAHLZEIT 1>&2", osmo_environment_whitelist, addl_env); + OSMO_ASSERT(pid > 0); + for (i = 0; i < 10; i++) { + sleep(1); + rc = waitpid(pid, NULL, WNOHANG); + if (rc == pid) { + printf("Success!\n"); + return; + } + } + printf("ERROR: child didn't terminate within 10s\n"); +} + +int main(int argc, char **argv) +{ + test_env_filter(); + test_env_append(); + test_close_fd(); + test_system_nowait(); + + exit(0); +} diff --git a/tests/exec/exec_test.err b/tests/exec/exec_test.err new file mode 100644 index 00000000..4edc61d6 --- /dev/null +++ b/tests/exec/exec_test.err @@ -0,0 +1 @@ +MAHLZEIT=spaet diff --git a/tests/exec/exec_test.ok b/tests/exec/exec_test.ok new file mode 100644 index 00000000..45a20f07 --- /dev/null +++ b/tests/exec/exec_test.ok @@ -0,0 +1,46 @@ + +==== osmo_environment_filter ==== +Input Environment: + FOO=1 + BAR=2 + USER=mahlzeit + BAZ=3 + SHELL=/bin/sh +Input Whitelist: + SHELL + USER +Output Environment (3): + USER=mahlzeit + SHELL=/bin/sh +Testing for NULL out +Testing for zero-length out +Testing for one-length out +Testing for no filter +Testing for no input +Success! + +==== osmo_environment_append ==== +Input Environment: + FOO=a + BAR=b + BAZ=c +Input Addition: + MAHL=zeit + GSM=global + UMTS=universal + LTE=evolved +Output Environment (8) + FOO=a + BAR=b + BAZ=c + MAHL=zeit + GSM=global + UMTS=universal + LTE=evolved +Success! + +==== osmo_close_all_fds_above ==== +Success! + +==== osmo_system_nowait ==== +Success! diff --git a/tests/testsuite.at b/tests/testsuite.at index c231b964..cb83ab91 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -362,3 +362,10 @@ AT_KEYWORDS([context]) cat $abs_srcdir/context/context_test.ok > expout AT_CHECK([$abs_top_builddir/tests/context/context_test], [0], [expout], [ignore]) AT_CLEANUP + +AT_SETUP([exec]) +AT_KEYWORDS([exec]) +cat $abs_srcdir/exec/exec_test.ok > expout +cat $abs_srcdir/exec/exec_test.err > experr +AT_CHECK([$abs_top_builddir/tests/exec/exec_test], [0], [expout], [experr]) +AT_CLEANUP -- cgit v1.2.3