[PATCH] start_command: reset disposition of all signals in child

["Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

In order to avoid invoking a signal handler in the child process between
fork() and exec(), start_command() blocks all signals before calling
fork and then in the child resets the disposition of all signals that
are not ignored to SIG_DFL before unblocking them. The exception for
ignored signals seems to been inspired by ruby's process handling[1]
based on the misconception that execve() will reset them to
SIG_DFL. Unfortunately this is not the case [2] and any signals that are
ignored in the parent will default to being ignored by the program
executed by start_command().

When git ignores SIGPIPE before forking a child process it is to stop
git from being killed if the child exits while git is writing to the
child's stdin. We do not want to ignore SIGPIPE in the child. When git
ignores SIGINT and SIGQUIT before forking a child process it is to stop
git from being killed if the user interrupts the child with Ctrl-C or
Ctrl-\ we do not want the child to ignore those signals [3].
Fortunately the fix is easy - reset the disposition of all signals
regardless of their previous disposition.

[1] https://lore.kernel.org/git/20170413211428.GA5961@whir/

[2] The man page for execve(2) states:

        POSIX.1 specifies that the dispositions of any signals that are
	ignored or set to the default are left unchanged.  POSIX.1
	specifies one exception: if SIGCHLD is being ignored, then an
	implementation may leave the disposition unchanged or reset it
	to the default; Linux does the former.

    Page 579 of "The Linux Programming Interface" notes:

        SUSv3 recommends that signals should not be blokced or ignored
	across an exec() of an arbitrary program. Here, "arbitrary"
	means a program that we did not write.

[3] This is really a work-around for not moving the child into its own
    process group and changing the foreground process group of the
    controlling terminal.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
    start_command: reset disposition of all signals in child
    
    As an aside I wonder if we ought to add an option to ignore SIGPIPE when
    stdin is redirected and possibly turn it on by default.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1582%2Fphillipwood%2Fstart-command-dont-ignore-signals-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1582/phillipwood/start-command-dont-ignore-signals-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1582

 run-command.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/run-command.c b/run-command.c
index a558042c876..765775a1f42 100644
--- a/run-command.c
+++ b/run-command.c
@@ -823,11 +823,8 @@ fail_pipe:
 		 * restore default signal handlers here, in case
 		 * we catch a signal right before execve below
 		 */
-		for (sig = 1; sig < NSIG; sig++) {
-			/* ignored signals get reset to SIG_DFL on execve */
-			if (signal(sig, SIG_DFL) == SIG_IGN)
-				signal(sig, SIG_IGN);
-		}
+		for (sig = 1; sig < NSIG; sig++)
+			signal(sig, SIG_DFL);
 
 		if (sigprocmask(SIG_SETMASK, &as.old, NULL) != 0)
 			child_die(CHILD_ERR_SIGPROCMASK);

base-commit: 1fc548b2d6a3596f3e1c1f8b1930d8dbd1e30bf3
-- 
gitgitgadget