From: Philip Guenther <guenther@gmail.com>
Subject: Re: readpassphrase(3): avoid spin on ignored signals
To: Damien Miller <djm@mindrot.org>
Cc: tech@openbsd.org
Date: Mon, 14 Sep 2026 23:57:41 -0700

The defect here seems to be that this code overrides a SIG_IGN
disposition for any signal. At least ksh, pax, dump, and restore do
*not* do this: only setting a signal handler if the disposition isn't
SIG_IGN.  c.f. setup_sig() in pax.

(POSIX actually says, iirc, this should be the default behavior of
utilities if not explicitly stated as doing otherwise in their
description.  I pondered proposing a libc API for this, but I wasn't
finding my diffs productive so, TODO list, oh well)

On Mon, Sep 14, 2026 at 11:16 PM Damien Miller <djm@mindrot.org> wrote:
>
> Hi,
>
> This was a bug report made to portable OpenSSH via
> https://bugzilla.mindrot.org/attachment.cgi?id=3983
>
> > When ssh-add (or any OpenSSH tool) is run as a background process
> > without a controlling TTY and with SIGTTOU or SIGTTIN set to SIG_IGN
> > (e.g. from a systemd service or a daemon that ignores job-control
> > signals), readpassphrase() spins at near-100% CPU indefinitely.
> >
> > Root cause: after readpassphrase() installs its own signal handlers,
> > reads from the TTY cause SIGTTOU or SIGTTIN to fire. The handler sets
> > signo[SIGTTIN/SIGTTOU]=1 and the read returns EINTR. readpassphrase then
> > restores the original SIG_IGN disposition and re-raises those signals.
> > Because SIG_IGN was restored, kill(getpid(), SIGTTIN) is a no-op, but
> > need_restart is still set to 1, causing an unconditional "goto restart".
> > The next iteration opens /dev/tty again, writes the prompt to stderr,
> > hits the same SIGTTIN/SIGTTOU, and the loop repeats forever.
> >
> > Fix: before setting need_restart for SIGTSTP/SIGTTIN/SIGTTOU, check
> > whether the original (saved) handler was SIG_IGN. If it was, do not
> > restart, because restarting would never make progress: the signal will
> > be ignored indefinitely and the process can never become the terminal
> > foreground group.
> >
> > This is the ssh-add equivalent of the fix applied to the ssh client for
> > Launchpad bug #1646813 (Ubuntu openssh 1:7.4p1-5).
>
> Ok?
>
> Index: lib/libc/gen/readpassphrase.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/gen/readpassphrase.c,v
> diff -u -p -r1.29 readpassphrase.c
> --- lib/libc/gen/readpassphrase.c       10 Mar 2026 16:27:33 -0000      1.29
> +++ lib/libc/gen/readpassphrase.c       15 Sep 2026 06:12:21 -0000
> @@ -164,7 +164,28 @@ restart:
>                         case SIGTSTP:
>                         case SIGTTIN:
>                         case SIGTTOU:
> -                               need_restart = 1;
> +                               /*
> +                                * Do not restart if the original handler for
> +                                * this signal was SIG_IGN.  Restarting in that
> +                                * case would spin forever: the signal is
> +                                * re-raised but immediately discarded (SIG_IGN
> +                                * was restored above), so the tty condition
> +                                * never resolves and readpassphrase loops at
> +                                * ~100% CPU.  This matches the behaviour of a
> +                                * background process that has no way to acquire
> +                                * the terminal (e.g. ssh-add running without a
> +                                * controlling TTY or with SIGTTOU/SIGTTIN
> +                                * ignored by its parent).
> +                                */
> +                               if (i == SIGTSTP &&
> +                                   savetstp.sa_handler != SIG_IGN)
> +                                       need_restart = 1;
> +                               if (i == SIGTTIN &&
> +                                   savettin.sa_handler != SIG_IGN)
> +                                       need_restart = 1;
> +                               if (i == SIGTTOU &&
> +                                   savettou.sa_handler != SIG_IGN)
> +                                       need_restart = 1;
>                         }
>                 }
>         }
>