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 5b42418..d918391 100644 (file)
--- 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 983e5da..81ae21f 100644 (file)
--- 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 ce31ef8..e959a55 100644 (file)
--- 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 9d971f0..65dd8d6 100644 (file)
--- 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;
 }
 
 /*