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@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