From: deraadt <deraadt@openbsd.org>
Date: Sat, 16 Aug 2014 07:49:27 +0000 (+0000)
Subject: I found a number of interactive events which can cause signals, and go
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=d05c767ae30090ccdb9db8118b01a8ccd0a27066;p=openbsd

I found a number of interactive events which can cause signals, and go
down paths not previously marked as signal handled unsafe.  Try to clean
up a few of them especially regarding errno, mark others as unsafe, and
repair a few by avoiding stdio.  Glanced at by misc people in Slovenia,
but considered too risky before release..
---

diff --git a/usr.bin/ftp/cmds.c b/usr.bin/ftp/cmds.c
index 5b424182bf8..d918391edc5 100644
--- a/usr.bin/ftp/cmds.c
+++ b/usr.bin/ftp/cmds.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cmds.c,v 1.71 2012/10/15 21:20:05 bluhm Exp $	*/
+/*	$OpenBSD: cmds.c,v 1.72 2014/08/16 07:49:27 deraadt Exp $	*/
 /*	$NetBSD: cmds.c,v 1.27 1997/08/18 10:20:15 lukem Exp $	*/
 
 /*
@@ -79,6 +79,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <errno.h>
 
 #include "ftp_var.h"
 #include "pathnames.h"
@@ -1326,6 +1327,7 @@ jmp_buf abortprox;
 void
 proxabort(int signo)
 {
+	int save_errno = errno;
 
 	alarmtimer(0);
 	if (!proxy) {
@@ -1338,6 +1340,7 @@ proxabort(int signo)
 		proxflag = 0;
 	}
 	pswitch(0);
+	errno = save_errno;
 	longjmp(abortprox, 1);
 }
 
diff --git a/usr.bin/ftp/ftp.c b/usr.bin/ftp/ftp.c
index 983e5da036c..81ae21f0da1 100644
--- a/usr.bin/ftp/ftp.c
+++ b/usr.bin/ftp/ftp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ftp.c,v 1.86 2014/05/20 01:25:23 guenther Exp $	*/
+/*	$OpenBSD: ftp.c,v 1.87 2014/08/16 07:49:27 deraadt Exp $	*/
 /*	$NetBSD: ftp.c,v 1.27 1997/08/18 10:20:23 lukem Exp $	*/
 
 /*
@@ -317,11 +317,13 @@ bad:
 void
 cmdabort(int signo)
 {
+	int save_errno = errno;
 
 	alarmtimer(0);
-	putc('\n', ttyout);
-	(void)fflush(ttyout);
+	(void) write(fileno(ttyout), "\n\r", 2);
 	abrtflag++;
+
+	errno = save_errno;
 	if (ptflag)
 		longjmp(ptabort, 1);
 }
@@ -575,12 +577,15 @@ jmp_buf	sendabort;
 void
 abortsend(int signo)
 {
-
+	int save_errno = errno;
 	alarmtimer(0);
 	mflag = 0;
 	abrtflag = 0;
-	fputs("\nsend aborted\nwaiting for remote to finish abort.\n", ttyout);
-	(void)fflush(ttyout);
+#define MSG "\nsend aborted\nwaiting for remote to finish abort.\n"
+	(void) write(fileno(ttyout), MSG, strlen(MSG));
+#undef MSG
+
+	errno = save_errno;
 	longjmp(sendabort, 1);
 }
 
@@ -2040,14 +2045,13 @@ jmp_buf forceabort;
 static void
 abortforce(int signo)
 {
-	fputs("Forced abort.  The connection will be closed.\n", ttyout);
-	(void)fflush(ttyout);
+	int save_errno = errno;
 
-	if (cout) {
-		(void)fclose(cout);
-	}
-	cout = NULL;
+#define MSG	"Forced abort.  The connection will be closed.\n"
+	(void) write(fileno(ttyout), MSG, strlen(MSG));
+#undef MSG
 
+	errno = save_errno;
 	longjmp(forceabort, 1);
 }
 
@@ -2061,6 +2065,8 @@ abort_remote(FILE *din)
 	sig_t oldintr;
 
 	if (cout == NULL || setjmp(forceabort)) {
+		if (cout)
+			fclose(cout);
 		warnx("Lost control connection for abort.");
 		if (ptabflg)
 			code = -1;
diff --git a/usr.bin/ftp/small.c b/usr.bin/ftp/small.c
index ce31ef8b535..e959a556081 100644
--- a/usr.bin/ftp/small.c
+++ b/usr.bin/ftp/small.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: small.c,v 1.1 2009/05/05 19:35:30 martynas Exp $	*/
+/*	$OpenBSD: small.c,v 1.2 2014/08/16 07:49:27 deraadt Exp $	*/
 /*	$NetBSD: cmds.c,v 1.27 1997/08/18 10:20:15 lukem Exp $	*/
 
 /*
@@ -77,6 +77,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <errno.h>
 
 #include "ftp_var.h"
 #include "pathnames.h"
@@ -301,15 +302,21 @@ freegetit:
 void
 mabort(int signo)
 {
+	int save_errno = errno;
+
 	alarmtimer(0);
-	putc('\n', ttyout);
-	(void)fflush(ttyout);
+	(void) write(fileno(ttyout), "\n\r", 2);
 #ifndef SMALL
-	if (mflag && fromatty)
-		if (confirm(mname, NULL))
+	if (mflag && fromatty) {
+		/* XXX signal race, crazy unbelievable stdio misuse */
+		if (confirm(mname, NULL)) {
+			errno = save_errno;
 			longjmp(jabort, 1);
+		}
+	}
 #endif /* !SMALL */
 	mflag = 0;
+	errno = save_errno;
 	longjmp(jabort, 1);
 }
 
diff --git a/usr.bin/ftp/util.c b/usr.bin/ftp/util.c
index 9d971f08806..65dd8d6bce5 100644
--- a/usr.bin/ftp/util.c
+++ b/usr.bin/ftp/util.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: util.c,v 1.66 2014/01/29 16:58:21 dcoppa Exp $	*/
+/*	$OpenBSD: util.c,v 1.67 2014/08/16 07:49:27 deraadt Exp $	*/
 /*	$NetBSD: util.c,v 1.12 1997/08/18 10:20:27 lukem Exp $	*/
 
 /*-
@@ -944,17 +944,21 @@ ptransfer(int siginfo)
 	meg = 0;
 	if (bs > (1024 * 1024))
 		meg = 1;
+
+	/* XXX floating point printf in signal handler */
 	(void)snprintf(buf, sizeof(buf),
 	    "%lld byte%s %s in %.2f seconds (%.2f %sB/s)\n",
 	    (long long)bytes, bytes == 1 ? "" : "s", direction, elapsed,
 	    bs / (1024.0 * (meg ? 1024.0 : 1.0)), meg ? "M" : "K");
-	if (siginfo && bytes > 0 && elapsed > 0.0 && filesize >= 0
-	    && bytes + restart_point <= filesize) {
+
+	if (siginfo && bytes > 0 && elapsed > 0.0 && filesize >= 0 &&
+	    bytes + restart_point <= filesize) {
 		remaining = (int)((filesize - restart_point) /
-				  (bytes / elapsed) - elapsed);
+		    (bytes / elapsed) - elapsed);
 		hh = remaining / 3600;
 		remaining %= 3600;
-			/* "buf+len(buf) -1" to overwrite \n */
+
+		/* "buf+len(buf) -1" to overwrite \n */
 		snprintf(buf + strlen(buf) - 1, sizeof(buf) - strlen(buf),
 		    "  ETA: %02d:%02d:%02d\n", hh, remaining / 60,
 		    remaining % 60);
@@ -1028,12 +1032,14 @@ setttywidth(int signo)
 void
 alarmtimer(int wait)
 {
+	int save_errno = errno;
 	struct itimerval itv;
 
 	itv.it_value.tv_sec = wait;
 	itv.it_value.tv_usec = 0;
 	itv.it_interval = itv.it_value;
 	setitimer(ITIMER_REAL, &itv, NULL);
+	errno = save_errno;
 }
 
 /*