openssh bugs - Jan 2010 - [Bug 1692] New: sshd sometimes dies when sent multiple SIGHUPs in quick succession

https://bugzilla.mindrot.org/show_bug.cgi?id=1692

           Summary: sshd sometimes dies when sent multiple SIGHUPs in
                    quick succession
           Product: Portable OpenSSH
           Version: 5.3p1
          Platform: Other
               URL: https://bugs.launchpad.net/ubuntu/+source/openssh/+bug
                    /497781
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: sshd
        AssignedTo: unassigned-bugs at mindrot.org
        ReportedBy: cjwatson at debian.org


Created an attachment (id=1766)
 --> (https://bugzilla.mindrot.org/attachment.cgi?id=1766)
ignore SIGHUP across sshd re-exec window

In Ubuntu bug #497781, "PierreF" reported that it's sometimes
possible
to end up with no sshd running if you send it two SIGHUP signals in
quick succession, which can sometimes happen due to configuration of
networking scripts. Although I haven't been able to reproduce this
myself so far, I think this is because SIGHUP is reset to the default
action by execve(), and sshd's handler isn't reinstalled until shortly
after the exec, so there's a window when it's simply set to the default
action of terminating the process.

If this hypothesis is correct, which I think is likely, then the
attached patch should fix it by ignoring SIGHUP across the exec window.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1692

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at zip.com.au

--- Comment #1 from Darren Tucker <dtucker at zip.com.au> 2010-01-06
10:33:36 EST ---
Seems like a reasonable hypothesis, but I don't see the patch making
any difference.

The execv will result in an entirely new process address space
(including address layout randomization on platforms that have it) and
the disposition of the old process' signal handlers will be irrelevant.
 You'd still have a window until the signal handler is reinstalled
where the default action of SIGHUP would kill sshd.

You could minimize this window by moving the "signal(SIGCHLD,
main_sigchld_handler)" to the start of main().  This wouldn't eliminate
the window but it would shrink it a lot (particularly because the
generation of the protocol 1 ephemeral server key would no longer be in
the window).

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1692

--- Comment #2 from Colin Watson <cjwatson at debian.org> 2010-01-06
12:18:01 EST ---
That isn't how execve() works, though. Signal dispositions are
inherited, not reset by the action of loading the new process image.
Lots of things wouldn't work if things were the way you posit - for
example, nohup(1) would be entirely non-functional.

Here's a transcript of a test demonstrating that ignoring SIGHUP before
execve() is effective. Does this convince you?

$ cat t.c
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <signal.h>
typedef void (*sighandler_t)(int);
extern char **environ;
int main(int argc, char **argv) {
    if (getenv("SECOND_TIME")) {
        sighandler_t prev = signal(SIGHUP, SIG_DFL);
        if (prev == SIG_IGN) {
            printf("SIGHUP was SIG_IGN\n");
        } else {
            printf("SIGHUP was not SIG_IGN\n");
        }
        exit(0);
    } else {
        setenv("SECOND_TIME", "1", 1);
        if (getenv("IGNORE_SIGHUP"))
            signal(SIGHUP, SIG_IGN);
        execve(argv[0], argv, environ);
    }
}
$ make t
cc     t.c   -o t
$ ./t
SIGHUP was not SIG_IGN
$ IGNORE_SIGHUP=1 ./t
SIGHUP was SIG_IGN

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1692

--- Comment #3 from Colin Watson <cjwatson at debian.org> 2010-01-06
12:19:36 EST ---
BTW, I agree that ASLR makes a difference when the signal handler is a
function, but in this case that is not so. SIG_IGN is (typically;
certainly on Linux, I imagine on other Unixes too) just a constant and
not affected by ASLR.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1692

--- Comment #4 from Colin Watson <cjwatson at debian.org> 2010-01-06
12:21:27 EST ---
I notice that the original reporter of
https://bugs.launchpad.net/ubuntu/+source/openssh/+bug/497781 confirmed
in a comment on that bug that my patch appears to fix the problem for
him.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1692

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |1626

--- Comment #5 from Darren Tucker <dtucker at zip.com.au> 2010-01-09
21:01:02 EST ---
Fair enough, I've never actually thought about how nohup works :-)  

Adding to the list for 5.4.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1692

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #6 from Darren Tucker <dtucker at zip.com.au> 2010-01-09
22:19:41 EST ---
Patch applied, thanks.  It will be in the 5.4 release.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1692

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #7 from Darren Tucker <dtucker at zip.com.au> 2010-03-26
10:51:34 EST ---
With the release of 5.4p1, this bug is now considered closed.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.