Change in libosmocore[master]: Introduce helper functions for safe fork+exec of processes

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

laforge gerrit-no-reply at lists.osmocom.org
Wed Dec 18 12:13:22 UTC 2019


laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/16619 )

Change subject: Introduce helper functions for safe fork+exec of processes
......................................................................

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
---
M configure.ac
M include/Makefile.am
A include/osmocom/core/exec.h
M src/Makefile.am
A src/exec.c
M tests/Makefile.am
A tests/exec/exec_test.c
A tests/exec/exec_test.err
A tests/exec/exec_test.ok
M tests/testsuite.at
10 files changed, 487 insertions(+), 4 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved



diff --git a/configure.ac b/configure.ac
index e45ec9f..1056a0a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -282,7 +282,7 @@
 	)],
 	[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 @@
 	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 dc6eaa7..b341ee3 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -23,6 +23,7 @@
                        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 0000000..6bbd352
--- /dev/null
+++ b/include/osmocom/core/exec.h
@@ -0,0 +1,28 @@
+#pragma once
+/* (C) 2019 by Harald Welte <laforge at gnumonks.org>
+ *
+ * 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 9943281..eeb3f7d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -27,6 +27,7 @@
 			 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 0000000..a9d8ce0
--- /dev/null
+++ b/src/exec.c
@@ -0,0 +1,238 @@
+/* (C) 2019 by Harald Welte <laforge at gnumonks.org>
+ *
+ * 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 <unistd.h>
+
+#include <errno.h>
+#include <string.h>
+
+#include <stdio.h>
+#include <dirent.h>
+#include <sys/types.h>
+
+#include <osmocom/core/logging.h>
+#include <osmocom/core/utils.h>
+#include <osmocom/core/exec.h>
+
+/*! 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 3a3ea37..bf7017b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -60,8 +60,10 @@
 	$(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 @@
 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 @@
 	     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 0000000..5f4b460
--- /dev/null
+++ b/tests/exec/exec_test.c
@@ -0,0 +1,155 @@
+#include <osmocom/core/utils.h>
+#include <osmocom/core/exec.h>
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <errno.h>
+
+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 0000000..4edc61d
--- /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 0000000..45a20f0
--- /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 c231b96..cb83ab9 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -362,3 +362,10 @@
 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

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/16619
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ib24ac8a083db32e55402ce496a5eabd8749cc888
Gerrit-Change-Number: 16619
Gerrit-PatchSet: 5
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191218/ecbda25c/attachment.htm>


More information about the gerrit-log mailing list