[PATCH] Improved run_program
by Laine Stump
The original run_program simply called system(3), which was expedient,
but failed to do things like close open file descriptors prior to
exec'ing the child binary. This particular omission caused complaints
when SELinux audited for leaked file descriptors during exec.
The new run program does the fork/exec manually, and in between it
closes all open file descriptors in the child process, as well as
clearing and restoring the signal mask to prevent a potential race
condition when killing off the parent's signal handlers in the child
process.
The code was heavily inspired by virRun() in libvirt, but is greatly
simplified, since netcf doesn't (currently) need all the facilities of
virRun().
---
configure.ac | 7 +++
src/netcf.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 133 insertions(+), 13 deletions(-)
diff --git a/configure.ac b/configure.ac
index 321cd22..9f002d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -38,6 +38,13 @@ NETCF_CHECK_READLINE
NETCF_LIBDEPS=$(echo $LIBAUGEAS_LIBS $LIBEXSLT_LIBS $LIBXSLT_LIBS $LIBXML_LIBS $LIBNL_LIBS)
AC_SUBST([NETCF_LIBDEPS])
+AC_CHECK_HEADER([pthread.h],
+ [AC_CHECK_LIB([pthread],[pthread_join],[
+ AC_DEFINE([HAVE_LIBPTHREAD],[],[Define if pthread (-lpthread)])
+ AC_DEFINE([HAVE_PTHREAD_H],[],[Define if <pthread.h>])
+ LIBS="-lpthread $LIBS"
+ ])])
+
AC_OUTPUT(Makefile \
gnulib/lib/Makefile \
gnulib/tests/Makefile \
diff --git a/src/netcf.c b/src/netcf.c
index 7cfcc6a..7c642a3 100644
--- a/src/netcf.c
+++ b/src/netcf.c
@@ -27,6 +27,10 @@
#include <string.h>
#include <stdarg.h>
#include <stdio.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <signal.h>
+#include <errno.h>
#include "safe-alloc.h"
#include "internal.h"
@@ -262,25 +266,134 @@ int ncf_put_aug(struct netcf *ncf, const char *aug_xml, char **ncf_xml) {
* Internal helpers
*/
+static int
+exec_program(struct netcf *ncf,
+ const char *const*argv,
+ const char *commandline,
+ pid_t *pid)
+{
+ sigset_t oldmask, newmask;
+ struct sigaction sig_action;
+ char errbuf[128];
+
+ /* commandline is only used for error reporting */
+ if (commandline == NULL)
+ commandline = argv[0];
+ /*
+ * Need to block signals now, so that child process can safely
+ * kill off caller's signal handlers without a race.
+ */
+ sigfillset(&newmask);
+ if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) {
+ report_error(ncf, NETCF_EEXEC,
+ "failed to set signal mask while forking for '%s': %s",
+ commandline, strerror_r(errno, errbuf, sizeof(errbuf)));
+ goto error;
+ }
+
+ *pid = fork();
+
+ ERR_THROW(*pid < 0, ncf, EEXEC,
+ "failed to set signal mask while forking for '%s': %s",
+ commandline, strerror_r(errno, errbuf, sizeof(errbuf)));
+
+ if (*pid) { /* parent */
+ /* Restore our original signal mask now that the child is
+ safely running */
+ ERR_THROW(pthread_sigmask(SIG_SETMASK, &oldmask, NULL) != 0,
+ ncf, EEXEC,
+ "failed to restore signal mask while forking for '%s': %s",
+ commandline, strerror_r(errno, errbuf, sizeof(errbuf)));
+ return 0;
+ }
+
+ /* child */
+
+ /* Clear out all signal handlers from parent so nothing unexpected
+ can happen in our child once we unblock signals */
+
+ sig_action.sa_handler = SIG_DFL;
+ sig_action.sa_flags = 0;
+ sigemptyset(&sig_action.sa_mask);
+
+ int i;
+ for (i = 1; i < NSIG; i++) {
+ /* Only possible errors are EFAULT or EINVAL
+ The former wont happen, the latter we
+ expect, so no need to check return value */
+
+ sigaction(i, &sig_action, NULL);
+ }
+
+ /* Unmask all signals in child, since we've no idea what the
+ caller's done with their signal mask and don't want to
+ propagate that to children */
+ sigemptyset(&newmask);
+ if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) {
+ report_error(ncf, NETCF_EEXEC,
+ "failure while unmasking signals in child for '%s': %s",
+ commandline, strerror_r(errno, errbuf, sizeof(errbuf)));
+ _exit(1);
+ }
+
+ /* close all open file descriptors */
+ int openmax = sysconf (_SC_OPEN_MAX);
+ for (i = 3; i < openmax; i++)
+ close(i);
+
+ execvp(argv[0], (char **) argv);
+
+ /* if execvp() returns, it has failed */
+ report_error(ncf, NETCF_EEXEC, "Failed to execute '%s': %s",
+ commandline, strerror_r(errno, errbuf, sizeof(errbuf)));
+ _exit(1);
+
+error:
+ /* This is cleanup of parent process only - child
+ should never jump here on error */
+ return -1;
+}
+
+/**
+ * Run a command without using the shell.
+ *
+ * return 0 if the command run and exited with 0 status; Otherwise
+ * return -1
+ *
+ */
int run_program(struct netcf *ncf, const char *const *argv) {
- int status, success;
- char *argv_str = argv_to_string(argv);
+ pid_t childpid;
+ int exitstatus, waitret;
+ char *argv_str;
int ret = -1;
+ char errbuf[128];
- /* BIG FIXME!!! Before any general release, this *must* be
- * replaced with a call to a function similar to libVirt's
- * virRun(), and if there is an error returned, anything the
- * program produced on stderr or stdout should be placed in
- * ncf->errdetails.
- */
- status = system(argv_str);
- success = WIFEXITED(status) && WEXITSTATUS(status) == 0;
- ERR_THROW(!success, ncf, EEXEC,
- "Running '%s' failed with error code %d",
- argv_str == NULL ? argv[0] : argv_str, WEXITSTATUS(status));
+ argv_str = argv_to_string(argv);
+ ERR_NOMEM(argv_str == NULL, ncf);
+ exec_program(ncf, argv, argv_str, &childpid);
+ ERR_BAIL(ncf);
+
+ while ((waitret = waitpid(childpid, &exitstatus, 0) == -1) &&
+ errno == EINTR) {
+ /* empty loop */
+ }
+
+ ERR_THROW(waitret == -1, ncf, EEXEC,
+ "Failed waiting for completion of '%s': %s",
+ argv_str, strerror_r(errno, errbuf, sizeof(errbuf)));
+ ERR_THROW(!WIFEXITED(exitstatus) && WIFSIGNALED(exitstatus), ncf, EEXEC,
+ "'%s' exited abnormally with signal: %d",
+ argv_str, WTERMSIG(exitstatus));
+ ERR_THROW(!WIFEXITED(exitstatus), ncf, EEXEC,
+ "'%s' exited improperly: %d",
+ argv_str, WEXITSTATUS(exitstatus));
+ ERR_THROW(WEXITSTATUS(exitstatus) != 0, ncf, EEXEC,
+ "Running '%s' failed with exit code %d",
+ argv_str, WEXITSTATUS(exitstatus));
ret = 0;
+
error:
FREE(argv_str);
return ret;
--
1.6.6.1
14 years
- 2
- 3
[PATCH] Set FD_CLOEXEC on netlink's file descriptor.
by Laine Stump
This protects against an application that uses netcf and may exec a
program without explicitly closing all open file descriptors in the
child process.
---
src/dutil.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/dutil.c b/src/dutil.c
index dbb43ef..2a6eaa0 100644
--- a/src/dutil.c
+++ b/src/dutil.c
@@ -519,6 +519,9 @@ int netlink_init(struct netcf *ncf) {
}
nl_cache_mngt_provide(ncf->driver->addr_cache);
+ int netlink_fd = nl_socket_get_fd(ncf->driver->nl_sock);
+ if (netlink_fd >= 0)
+ fcntl(netlink_fd, F_SETFD, FD_CLOEXEC);
return 0;
error:
--
1.6.6.1
14 years, 1 month
- 1
- 0
- ← Newer
- 1
- Older →