Remove explicit kill of privsep preauth child's PID in SIGALRM handler.
authordtucker <dtucker@openbsd.org>
Tue, 1 Feb 2022 07:57:32 +0000 (07:57 +0000)
committerdtucker <dtucker@openbsd.org>
Tue, 1 Feb 2022 07:57:32 +0000 (07:57 +0000)
It's no longer needed since the child will get terminated by the SIGTERM
to the process group that cleans up any auth helpers, it simplifies
the signal handler and removes the risk of a race when updating the PID.
Based on analysis by HerrSpace in github PR#289, ok djm@

usr.bin/ssh/sshd.c

index 3c21b3d..f86f83f 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.582 2021/11/18 03:07:59 djm Exp $ */
+/* $OpenBSD: sshd.c,v 1.583 2022/02/01 07:57:32 dtucker Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -332,12 +332,9 @@ main_sigchld_handler(int sig)
 static void
 grace_alarm_handler(int sig)
 {
-       if (use_privsep && pmonitor != NULL && pmonitor->m_pid > 0)
-               kill(pmonitor->m_pid, SIGALRM);
-
        /*
         * Try to kill any processes that we have spawned, E.g. authorized
-        * keys command helpers.
+        * keys command helpers or privsep children.
         */
        if (getpgid(0) == getpid()) {
                ssh_signal(SIGTERM, SIG_IGN);
@@ -345,13 +342,9 @@ grace_alarm_handler(int sig)
        }
 
        /* Log error and exit. */
-       if (use_privsep && pmonitor != NULL && pmonitor->m_pid <= 0)
-               cleanup_exit(255); /* don't log in privsep child */
-       else {
-               sigdie("Timeout before authentication for %s port %d",
-                   ssh_remote_ipaddr(the_active_state),
-                   ssh_remote_port(the_active_state));
-       }
+       sigdie("Timeout before authentication for %s port %d",
+           ssh_remote_ipaddr(the_active_state),
+           ssh_remote_port(the_active_state));
 }
 
 /* Destroy the host and server keys.  They will no longer be needed. */