dd(1): neutralize SIGINT while non-async-signal safe code is executing

making the SIGINT handler (the terminate() function) safe to execute at
any interruption moment.  This fixes a race in
5807f35c54 where SIGINT delivered right
after the check_terminate() but before a blocking syscall would not
cause abort.

Do it by setting the in_io flag around potentially blocking io syscalls.
If handler sees the flag, it terminates the program.  Otherwise,
termination is delegated to the before_io/after_io fences.

Reviewed by:	Andrew Gierth <andrew@tao146.riddles.org.uk>
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D40281

bin/dd/dd.c

@@ -99,8 +99,7 @@ main(int argc __unused, char *argv[])
 {
 	struct itimerval itv = { { 1, 0 }, { 1, 0 } }; /* SIGALARM every second, if needed */
 
-	(void)siginterrupt(SIGINT, 1);
-	(void)signal(SIGINT, terminate);
+	prepare_io();
 
 	(void)setlocale(LC_CTYPE, "");
 	jcl(argv);
@@ -158,9 +157,9 @@ setup(void)
 		iflags = 0;
 		if (ddflags & C_IDIRECT)
 			iflags |= O_DIRECT;
-		check_terminate();
+		before_io();
 		in.fd = open(in.name, O_RDONLY | iflags, 0);
-		check_terminate();
+		after_io();
 		if (in.fd == -1)
 			err(1, "%s", in.name);
 	}
@@ -197,17 +196,18 @@ setup(void)
 			oflags |= O_FSYNC;
 		if (ddflags & C_ODIRECT)
 			oflags |= O_DIRECT;
-		check_terminate();
+		before_io();
 		out.fd = open(out.name, O_RDWR | oflags, DEFFILEMODE);
-		check_terminate();
+		after_io();
 		/*
 		 * May not have read access, so try again with write only.
 		 * Without read we may have a problem if output also does
 		 * not support seeks.
 		 */
 		if (out.fd == -1) {
+			before_io();
 			out.fd = open(out.name, O_WRONLY | oflags, DEFFILEMODE);
-			check_terminate();
+			after_io();
 			out.flags |= NOREAD;
 			cap_rights_clear(&rights, CAP_READ);
 		}
@@ -424,9 +424,9 @@ dd_in(void)
 
 		in.dbrcnt = 0;
 fill:
-		check_terminate();
+		before_io();
 		n = read(in.fd, in.dbp + in.dbrcnt, in.dbsz - in.dbrcnt);
-		check_terminate();
+		after_io();
 
 		/* EOF */
 		if (n == 0 && in.dbrcnt == 0)
@@ -607,9 +607,9 @@ dd_out(int force)
 					pending = 0;
 				}
 				if (cnt) {
-					check_terminate();
+					before_io();
 					nw = write(out.fd, outp, cnt);
-					check_terminate();
+					after_io();
 					out.seek_offset = 0;
 				} else {
 					return;

bin/dd/extern.h

@@ -49,8 +49,9 @@ void progress(void);
 void summary(void);
 void sigalarm_handler(int);
 void siginfo_handler(int);
-void terminate(int);
-void check_terminate(void);
+void prepare_io(void);
+void before_io(void);
+void after_io(void);
 void unblock(void);
 void unblock_close(void);
 

bin/dd/misc.c

@@ -48,6 +48,7 @@ __FBSDID("$FreeBSD$");
 #include <inttypes.h>
 #include <libutil.h>
 #include <signal.h>
+#include <stdatomic.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -147,23 +148,58 @@ sigalarm_handler(int signo __unused)
 	need_progress = 1;
 }
 
-void
+static void terminate(int signo) __dead2;
+static void
 terminate(int signo)
 {
-
 	kill_signal = signo;
+	summary();
+	(void)fflush(stderr);
+	raise(kill_signal);
+	/* NOT REACHED */
+	_exit(1);
+}
+
+static sig_atomic_t in_io = 0;
+static sig_atomic_t sigint_seen = 0;
+
+static void
+sigint_handler(int signo __unused)
+{
+	atomic_signal_fence(memory_order_acquire);
+	if (in_io)
+		terminate(SIGINT);
+	sigint_seen = 1;
 }
 
 void
-check_terminate(void)
+prepare_io(void)
 {
+	struct sigaction sa;
+	int error;
 
-	if (kill_signal) {
-		summary();
-		(void)fflush(stderr);
-		signal(kill_signal, SIG_DFL);
-		raise(kill_signal);
-		/* NOT REACHED */
-		_exit(128 + kill_signal);
-	}
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_flags = SA_NODEFER | SA_RESETHAND;
+	sa.sa_handler = sigint_handler;
+	error = sigaction(SIGINT, &sa, 0);
+	if (error != 0)
+		err(1, "sigaction");
+}
+
+void
+before_io(void)
+{
+	in_io = 1;
+	atomic_signal_fence(memory_order_seq_cst);
+	if (sigint_seen)
+		terminate(SIGINT);
+}
+
+void
+after_io(void)
+{
+	in_io = 0;
+	atomic_signal_fence(memory_order_seq_cst);
+	if (sigint_seen)
+		terminate(SIGINT);
 }

bin/dd/position.c

@@ -191,10 +191,11 @@ pos_out(void)
 
 	/* Read it. */
 	for (cnt = 0; cnt < out.offset; ++cnt) {
-		check_terminate();
-		if ((n = read(out.fd, out.db, out.dbsz)) > 0)
+		before_io();
+		n = read(out.fd, out.db, out.dbsz);
+		after_io();
+		if (n > 0)
 			continue;
-		check_terminate();
 		if (n == -1)
 			err(1, "%s", out.name);
 
@@ -209,9 +210,9 @@ pos_out(void)
 			err(1, "%s", out.name);
 
 		while (cnt++ < out.offset) {
-			check_terminate();
+			before_io();
 			n = write(out.fd, out.db, out.dbsz);
-			check_terminate();
+			after_io();
 			if (n == -1)
 				err(1, "%s", out.name);
 			if (n != out.dbsz)