qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: [Qemu-devel] [PATCH v1 05/16] osdep: add qemu_fork() wrapper for safely handling signals
Date: Fri, 18 Sep 2015 14:18:59 +0100	[thread overview]
Message-ID: <1442582350-9179-6-git-send-email-berrange@redhat.com> (raw)
In-Reply-To: <1442582350-9179-1-git-send-email-berrange@redhat.com>

When using regular fork() the child process of course inherits
all the parents' signal handlers. If the child then proceeds
to close() any open file descriptors, it may break some of those
registered signal handlers. The child generally does not want to
ever run any of the signal handlers tha parent may have installed
in the short time before it exec's. The parent may also have blocked
various signals which the child process will want enabled.

This introduces a wrapper qemu_fork() that takes care to sanitize
signal handling across fork. Before forking it blocks all signals
in the parent thread. After fork returns, the parent unblocks the
signals and carries on as usual. The child, however, resets all the
signal handlers back to their defaults before it unblocks signals.
The child process can now exec the binary in a "clean" signal
environment.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qemu/osdep.h | 16 ++++++++++++
 util/oslib-posix.c   | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 util/oslib-win32.c   |  9 +++++++
 3 files changed, 96 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ab3c876..4f6af8d 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -67,6 +67,8 @@
 #include "sysemu/os-posix.h"
 #endif
 
+#include "qapi/error.h"
+
 #if defined(CONFIG_SOLARIS) && CONFIG_SOLARIS_VERSION < 10
 /* [u]int_fast*_t not in <sys/int_types.h> */
 typedef unsigned char           uint_fast8_t;
@@ -284,4 +286,18 @@ void os_mem_prealloc(int fd, char *area, size_t sz);
 
 int qemu_read_password(char *buf, int buf_size);
 
+/**
+ * qemu_fork:
+ *
+ * A version of fork that avoids signal handler race
+ * conditions that can lead to child process getting
+ * signals that are otherwise only expected by the
+ * parent. It also resets all signal handlers to the
+ * default settings.
+ *
+ * Returns 0 to child process, pid number to parent
+ * or -1 on failure.
+ */
+pid_t qemu_fork(Error **errp);
+
 #endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 3ae4987..e7b8245 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -482,3 +482,74 @@ int qemu_read_password(char *buf, int buf_size)
     printf("\n");
     return ret;
 }
+
+
+pid_t qemu_fork(Error **errp)
+{
+    sigset_t oldmask, newmask;
+    struct sigaction sig_action;
+    int saved_errno;
+    pid_t pid;
+
+    /*
+     * 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) {
+        error_setg_errno(errp, errno,
+                         "cannot block signals");
+        return -1;
+    }
+
+    pid = fork();
+    saved_errno = errno;
+
+    if (pid < 0) {
+        /* attempt to restore signal mask, but ignore failure, to
+         * avoid obscuring the fork failure */
+        (void)pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
+        error_setg_errno(errp, saved_errno,
+                         "cannot fork child process");
+        errno = saved_errno;
+        return -1;
+    } else if (pid) {
+        /* parent process */
+
+        /* Restore our original signal mask now that the child is
+         * safely running. Only documented failures are EFAULT (not
+         * possible, since we are using just-grabbed mask) or EINVAL
+         * (not possible, since we are using correct arguments).  */
+        (void)pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
+    } else {
+        /* child process */
+        size_t i;
+
+        /* 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);
+
+        for (i = 1; i < NSIG; i++) {
+            /* Only possible errors are EFAULT or EINVAL The former
+             * won't happen, the latter we expect, so no need to check
+             * return value */
+            (void)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) {
+            Error *local_err = NULL;
+            error_setg_errno(&local_err, errno,
+                             "cannot unblock signals");
+            error_report_err(local_err);
+            _exit(1);
+        }
+    }
+    return pid;
+}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 730a670..73dc35f 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -494,3 +494,12 @@ int qemu_read_password(char *buf, int buf_size)
     buf[i] = '\0';
     return 0;
 }
+
+
+pid_t qemu_fork(Error **errp)
+{
+    errno = ENOSYS;
+    error_setg_errno(errp, errno,
+                     "cannot fork child process");
+    return -1;
+}
-- 
2.4.3

  parent reply	other threads:[~2015-09-18 13:59 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-18 13:18 [Qemu-devel] [PATCH v1 00/16] Introduce I/O channels framework Daniel P. Berrange
2015-09-18 13:18 ` [Qemu-devel] [PATCH v1 01/16] sockets: add helpers for creating SocketAddress from a socket Daniel P. Berrange
2015-09-18 13:18 ` [Qemu-devel] [PATCH v1 02/16] sockets: move qapi_copy_SocketAddress into qemu-sockets.c Daniel P. Berrange
2015-09-18 13:18 ` [Qemu-devel] [PATCH v1 03/16] sockets: allow port to be NULL when listening on IP address Daniel P. Berrange
2015-09-18 13:18 ` [Qemu-devel] [PATCH v1 04/16] ui: convert VNC startup code to use SocketAddress Daniel P. Berrange
2015-09-18 13:18 ` Daniel P. Berrange [this message]
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 06/16] coroutine: move into libqemuutil.a library Daniel P. Berrange
2015-09-22 12:07   ` Paolo Bonzini
2015-09-24 14:46     ` Daniel P. Berrange
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 07/16] io: add abstract QIOChannel classes Daniel P. Berrange
2015-09-22 12:19   ` Paolo Bonzini
2015-09-22 12:24     ` Daniel P. Berrange
2015-09-22 12:28       ` Paolo Bonzini
2015-09-24 14:47         ` Daniel P. Berrange
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 08/16] io: add helper module for creating watches on FDs Daniel P. Berrange
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 09/16] io: pull Buffer code out of VNC module Daniel P. Berrange
2015-09-22 12:04   ` Paolo Bonzini
2015-09-22 12:20     ` Daniel P. Berrange
2015-09-22 12:21       ` Paolo Bonzini
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 10/16] io: add QIOTask class for async operations Daniel P. Berrange
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 11/16] io: add QIOChannelSocket class Daniel P. Berrange
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 12/16] io: add QIOChannelFile class Daniel P. Berrange
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 13/16] io: add QIOChannelTLS class Daniel P. Berrange
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 14/16] io: add QIOChannelWebsock class Daniel P. Berrange
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 15/16] io: add QIOChannelCommand class Daniel P. Berrange
2015-09-18 13:19 ` [Qemu-devel] [PATCH v1 16/16] io: add QIOChannelBuffer class Daniel P. Berrange
2015-09-25 10:15   ` Paolo Bonzini
2015-09-25 10:21     ` Daniel P. Berrange
2015-09-22 12:23 ` [Qemu-devel] [PATCH v1 00/16] Introduce I/O channels framework Paolo Bonzini
2015-09-24 14:51   ` Daniel P. Berrange
2015-09-25 10:18     ` Paolo Bonzini
2015-09-25 10:20       ` Daniel P. Berrange

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1442582350-9179-6-git-send-email-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).