Index | Thread | Search

From:
Damien Miller <djm@mindrot.org>
Subject:
Re: readpassphrase(3): avoid spin on ignored signals
To:
Philip Guenther <guenther@gmail.com>
Cc:
tech@openbsd.org
Date:
Tue, 15 Sep 2026 17:20:22 +1000

Download raw body.

Thread

On Mon, 14 Sep 2026, Philip Guenther wrote:

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

Something like the below?

> (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)

A nicer API than this hack would be good...


Index: readpassphrase.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/readpassphrase.c,v
diff -u -p -r1.29 readpassphrase.c
--- readpassphrase.c	10 Mar 2026 16:27:33 -0000	1.29
+++ readpassphrase.c	15 Sep 2026 07:20:14 -0000
@@ -36,6 +36,23 @@ static volatile sig_atomic_t signo[_NSIG
 
 static void handler(int);
 
+/* Like sigaction(2) but preserves SIG_IGN */
+static void
+sigaction_except_ign(int signum, struct sigaction *act, struct sigaction *old)
+{
+	/*
+	 * When reinstating: don't reinstate a SIG_IGN, because we already
+	 * did it.
+	 */
+	if (old == NULL && act->sa_handler == SIG_IGN)
+		return;
+
+	(void)sigaction(signum, act, old);
+	/* Reinstate SIG_IGN; we don't want to override this */
+	if (old->sa_handler == SIG_IGN)
+		(void)sigaction(signum, old, NULL);
+}
+
 char *
 readpassphrase(const char *prompt, char *buf, size_t bufsiz, int flags)
 {
@@ -99,15 +116,15 @@ restart:
 	sigemptyset(&sa.sa_mask);
 	sa.sa_flags = 0;		/* don't restart system calls */
 	sa.sa_handler = handler;
-	(void)sigaction(SIGALRM, &sa, &savealrm);
-	(void)sigaction(SIGHUP, &sa, &savehup);
-	(void)sigaction(SIGINT, &sa, &saveint);
-	(void)sigaction(SIGPIPE, &sa, &savepipe);
-	(void)sigaction(SIGQUIT, &sa, &savequit);
-	(void)sigaction(SIGTERM, &sa, &saveterm);
-	(void)sigaction(SIGTSTP, &sa, &savetstp);
-	(void)sigaction(SIGTTIN, &sa, &savettin);
-	(void)sigaction(SIGTTOU, &sa, &savettou);
+	sigaction_except_ign(SIGALRM, &sa, &savealrm);
+	sigaction_except_ign(SIGHUP, &sa, &savehup);
+	sigaction_except_ign(SIGINT, &sa, &saveint);
+	sigaction_except_ign(SIGPIPE, &sa, &savepipe);
+	sigaction_except_ign(SIGQUIT, &sa, &savequit);
+	sigaction_except_ign(SIGTERM, &sa, &saveterm);
+	sigaction_except_ign(SIGTSTP, &sa, &savetstp);
+	sigaction_except_ign(SIGTTIN, &sa, &savettin);
+	sigaction_except_ign(SIGTTOU, &sa, &savettou);
 
 	if (!(flags & RPP_STDIN))
 		(void)write(output, prompt, strlen(prompt));
@@ -141,15 +158,15 @@ restart:
 			continue;
 		signo[SIGTTOU] = sigttou;
 	}
-	(void)sigaction(SIGALRM, &savealrm, NULL);
-	(void)sigaction(SIGHUP, &savehup, NULL);
-	(void)sigaction(SIGINT, &saveint, NULL);
-	(void)sigaction(SIGQUIT, &savequit, NULL);
-	(void)sigaction(SIGPIPE, &savepipe, NULL);
-	(void)sigaction(SIGTERM, &saveterm, NULL);
-	(void)sigaction(SIGTSTP, &savetstp, NULL);
-	(void)sigaction(SIGTTIN, &savettin, NULL);
-	(void)sigaction(SIGTTOU, &savettou, NULL);
+	sigaction_except_ign(SIGALRM, &savealrm, NULL);
+	sigaction_except_ign(SIGHUP, &savehup, NULL);
+	sigaction_except_ign(SIGINT, &saveint, NULL);
+	sigaction_except_ign(SIGQUIT, &savequit, NULL);
+	sigaction_except_ign(SIGPIPE, &savepipe, NULL);
+	sigaction_except_ign(SIGTERM, &saveterm, NULL);
+	sigaction_except_ign(SIGTSTP, &savetstp, NULL);
+	sigaction_except_ign(SIGTTIN, &savettin, NULL);
+	sigaction_except_ign(SIGTTOU, &savettou, NULL);
 	if (input != STDIN_FILENO)
 		(void)close(input);