[libvirt] [PATCH v6] util: Set SIGPIPE to a no-op handler in virFork

Wang Yechao wang.yechao255 at zte.com.cn
Fri Nov 8 00:25:15 UTC 2019


Libvirtd has set SIGPIPE to ignored, and virFork resets all signal
handlers to the defaults. But child process may write logs to
stderr/stdout, that may generate SIGPIPE if journald has stopped.

So set SIGPIPE to a dummy no-op handler before unmask signals in
virFork(), and the handler will get reset to SIG_DFL when execve()
runs. Now we can delete sigaction() call entirely in virExec().

Signed-off-by: Wang Yechao <wang.yechao255 at zte.com.cn>

---
v3 patch:
https://www.redhat.com/archives/libvir-list/2019-October/msg00934.html

Changes in v4:
 -  don't block SIGPIPE, ignore it when invoke VIR_FORCE_CLOSE and virCommandMassClose

Changes in v5:
 - chang from SIG_IGN to a no-op handler in child process

Changes in v6:
 - add a comment and delete sigaction() call entirely in virExec
---
 src/util/vircommand.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 93b3dd2..8b10253 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -217,6 +217,8 @@ virCommandFDSet(virCommandPtr cmd,
 
 #ifndef WIN32
 
+static void virDummyHandler(int sig G_GNUC_UNUSED) {}
+
 /**
  * virFork:
  *
@@ -312,6 +314,14 @@ virFork(void)
             ignore_value(sigaction(i, &sig_action, NULL));
         }
 
+        /* Code that runs between fork & execve might trigger
+         * SIG_PIPE, so we must explicitly set that to a no-op
+         * handler. This handler will get reset to SIG_DFL when
+         * execve() runs
+         */
+        sig_action.sa_handler = virDummyHandler;
+        ignore_value(sigaction(SIGPIPE, &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 */
@@ -550,7 +560,6 @@ virExec(virCommandPtr cmd)
     g_autofree char *binarystr = NULL;
     const char *binary = NULL;
     int ret;
-    struct sigaction waxon, waxoff;
     g_autofree gid_t *groups = NULL;
     int ngroups;
 
@@ -718,21 +727,6 @@ virExec(virCommandPtr cmd)
         }
     }
 
-    /* virFork reset all signal handlers to the defaults.
-     * This is good for the child process, but our hook
-     * risks running something that generates SIGPIPE,
-     * so we need to temporarily block that again
-     */
-    memset(&waxoff, 0, sizeof(waxoff));
-    waxoff.sa_handler = SIG_IGN;
-    sigemptyset(&waxoff.sa_mask);
-    memset(&waxon, 0, sizeof(waxon));
-    if (sigaction(SIGPIPE, &waxoff, &waxon) < 0) {
-        virReportSystemError(errno, "%s",
-                             _("Could not disable SIGPIPE"));
-        goto fork_error;
-    }
-
     if (virProcessSetMaxMemLock(0, cmd->maxMemLock) < 0)
         goto fork_error;
     if (virProcessSetMaxProcesses(0, cmd->maxProcesses) < 0)
@@ -783,12 +777,6 @@ virExec(virCommandPtr cmd)
     if (virCommandHandshakeChild(cmd) < 0)
        goto fork_error;
 
-    if (sigaction(SIGPIPE, &waxon, NULL) < 0) {
-        virReportSystemError(errno, "%s",
-                             _("Could not re-enable SIGPIPE"));
-        goto fork_error;
-    }
-
     /* Close logging again to ensure no FDs leak to child */
     virLogReset();
 
-- 
1.8.3.1




More information about the libvir-list mailing list