Libguestfs - Jun 2017 - [PATCH] lib: direct, uml: Unblock SIGTERM in the hypervisor and recovery processes (RHBZ#1460338).

If SIGTERM is blocked in the main program, then it ends up still being
blocked in the subprocess after we fork.  This means that we cannot
kill qemu by sending SIGTERM to it.  This commit fixes the problem by
unblocking SIGTERM unconditionally after fork.

Thanks: wtfuzz on IRC for reporting and analysis.
---
 lib/launch-direct.c | 15 +++++++++++++++
 lib/launch-uml.c    | 11 +++++++++++
 2 files changed, 26 insertions(+)

diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index 8558cde5e..168ee367b 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -361,6 +361,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   const char *cpu_model;
   CLEANUP_FREE char *append = NULL;
   CLEANUP_FREE_STRING_LIST char **argv = NULL;
+  sigset_t sigset;
 
   /* At present you must add drives before starting the appliance.  In
    * future when we enable hotplugging you won't need to do this.
@@ -728,6 +729,13 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
       close_file_descriptors (fd > 2);
     }
 
+    /* Unblock the SIGTERM signal since we will need to send that to
+     * the subprocess (RHBZ#1460338).
+     */
+    sigemptyset (&sigset);
+    sigaddset (&sigset, SIGTERM);
+    sigprocmask (SIG_UNBLOCK, &sigset, NULL);
+
     /* Dump the command line (after setting up stderr above). */
     if (g->verbose)
       qemuopts_to_channel (qopts, stderr);
@@ -781,6 +789,13 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
        */
       close_file_descriptors (1);
 
+      /* Unblock the SIGTERM signal since we will need to respond to
+       * SIGTERM from the parent (RHBZ#1460338).
+       */
+      sigemptyset (&sigset);
+      sigaddset (&sigset, SIGTERM);
+      sigprocmask (SIG_UNBLOCK, &sigset, NULL);
+
       /* It would be nice to be able to put this in the same process
        * group as qemu (ie. setpgid (0, qemu_pid)).  However this is
        * not possible because we don't have any guarantee here that
diff --git a/lib/launch-uml.c b/lib/launch-uml.c
index eccc102f5..547ab5a69 100644
--- a/lib/launch-uml.c
+++ b/lib/launch-uml.c
@@ -147,6 +147,7 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)
   size_t i;
   struct hv_param *hp;
   char *term = getenv ("TERM");
+  sigset_t sigset;
 
   if (!uml_supported (g))
     return -1;
@@ -323,6 +324,11 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)
       close_file_descriptors (fd > 2 && fd != dsv[1]);
     }
 
+    /* RHBZ#1460338. */
+    sigemptyset (&sigset);
+    sigaddset (&sigset, SIGTERM);
+    sigprocmask (SIG_UNBLOCK, &sigset, NULL);
+
     /* Dump the command line (after setting up stderr above). */
     if (g->verbose)
       print_vmlinux_command_line (g, cmdline.argv);
@@ -369,6 +375,11 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)
        */
       close_file_descriptors (1);
 
+      /* RHBZ#1460338 */
+      sigemptyset (&sigset);
+      sigaddset (&sigset, SIGTERM);
+      sigprocmask (SIG_UNBLOCK, &sigset, NULL);
+
       /* It would be nice to be able to put this in the same process
        * group as vmlinux (ie. setpgid (0, vmlinux_pid)).  However
        * this is not possible because we don't have any guarantee here
-- 
2.13.0

On Saturday, 10 June 2017 19:33:12 CEST Richard W.M. Jones
wrote:
> If SIGTERM is blocked in the main program, then it ends up still being
> blocked in the subprocess after we fork.  This means that we cannot
> kill qemu by sending SIGTERM to it.  This commit fixes the problem by
> unblocking SIGTERM unconditionally after fork.
> 
> Thanks: wtfuzz on IRC for reporting and analysis.
> ---
LGTM, although a macro/function to avoid copy&pasting the same snippet
4 times would be better IMHO.

Thanks,
-- 
Pino Toscano