[src/trunk]: src/usr.bin/ftp ftp.c: improve signal handler restoration

[lukem <lukem%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/cbeab69e2641
branches:  trunk
changeset: 1011780:cbeab69e2641
user:      lukem <lukem%NetBSD.org@localhost>
date:      Sat Jul 11 02:19:31 2020 +0000

description:
ftp.c: improve signal handler restoration

Only invoke the old signal handler if it's a real signal handler
and not SIG_IGN, SIG_DFL, SIG_HOLD, or SIG_ERR, using new static
function issighandler().
Avoids an intermittent race condition with a null pointer
dereference via (*SIG_DFL)().
Bug class reported by Joyu Liao from Juniper Networks.

Use SIG_ERR instead of NULL as the indicator that a signal handler
hasn't been changed, so that SIG_DFL (equivalent to NULL)
will be restored.

diffstat:

 usr.bin/ftp/ftp.c     |  61 ++++++++++++++++++++++++++++++++------------------
 usr.bin/ftp/version.h |   4 +-
 2 files changed, 41 insertions(+), 24 deletions(-)

diffs (195 lines):

diff -r 4a59559a0b2c -r cbeab69e2641 usr.bin/ftp/ftp.c
--- a/usr.bin/ftp/ftp.c Sat Jul 11 00:39:53 2020 +0000
+++ b/usr.bin/ftp/ftp.c Sat Jul 11 02:19:31 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ftp.c,v 1.169 2020/06/08 01:33:27 lukem Exp $  */
+/*     $NetBSD: ftp.c,v 1.170 2020/07/11 02:19:31 lukem Exp $  */
 
 /*-
  * Copyright (c) 1996-2020 The NetBSD Foundation, Inc.
@@ -92,7 +92,7 @@
 #if 0
 static char sccsid[] = "@(#)ftp.c      8.6 (Berkeley) 10/27/94";
 #else
-__RCSID("$NetBSD: ftp.c,v 1.169 2020/06/08 01:33:27 lukem Exp $");
+__RCSID("$NetBSD: ftp.c,v 1.170 2020/07/11 02:19:31 lukem Exp $");
 #endif
 #endif /* not lint */
 
@@ -320,6 +320,17 @@
        errno = oerrno;
 }
 
+static int
+issighandler(sigfunc func)
+{
+       return (func != SIG_IGN &&
+               func != SIG_DFL &&
+#ifdef SIG_HOLD
+               func != SIG_HOLD &&
+#endif
+               func != SIG_ERR);
+}
+
 /*VARARGS*/
 int
 command(const char *fmt, ...)
@@ -359,8 +370,9 @@
        (void)fflush(cout);
        cpend = 1;
        r = getreply(!strcmp(fmt, "QUIT"));
-       if (abrtflag && oldsigint != SIG_IGN)
+       if (abrtflag && issighandler(oldsigint)) {
                (*oldsigint)(SIGINT);
+       }
        (void)xsignal(SIGINT, oldsigint);
        return (r);
 }
@@ -510,11 +522,14 @@
                (void)xsignal(SIGALRM, oldsigalrm);
                if (code == 421 || originalcode == 421)
                        lostpeer(0);
-               if (abrtflag && oldsigint != cmdabort && oldsigint != SIG_IGN)
+               if (abrtflag && oldsigint != cmdabort &&
+                   issighandler(oldsigint)) {
                        (*oldsigint)(SIGINT);
+               }
                if (timeoutflag && oldsigalrm != cmdtimeout &&
-                   oldsigalrm != SIG_IGN)
+                   issighandler(oldsigalrm)) {
                        (*oldsigalrm)(SIGINT);
+               }
                return (n - '0');
        }
 }
@@ -670,7 +685,7 @@
        FILE *volatile dout;
        int (*volatile closefunc)(FILE *);
        sigfunc volatile oldintr;
-       sigfunc volatile oldintp;
+       sigfunc volatile oldpipe;
        off_t volatile hashbytes;
        int hash_interval;
        const char *lmode;
@@ -697,8 +712,8 @@
        if (curtype != type)
                changetype(type, 0);
        closefunc = NULL;
-       oldintr = NULL;
-       oldintp = NULL;
+       oldintr = SIG_ERR;
+       oldpipe = SIG_ERR;
        lmode = "w";
        if (sigsetjmp(xferabort, 1)) {
                while (cpend)
@@ -712,7 +727,7 @@
                fin = stdin;
                progress = 0;
        } else if (*local == '|') {
-               oldintp = xsignal(SIGPIPE, SIG_IGN);
+               oldpipe = xsignal(SIGPIPE, SIG_IGN);
                fin = popen(local + 1, "r");
                if (fin == NULL) {
                        warn("Can't execute `%s'", local + 1);
@@ -786,7 +801,9 @@
        }
 
        progressmeter(-1);
-       oldintp = xsignal(SIGPIPE, SIG_IGN);
+       if (oldpipe == SIG_ERR) {
+               oldpipe = xsignal(SIGPIPE, SIG_IGN);
+       }
        hash_interval = (hash && (!progress || filesize < 0)) ? mark : 0;
 
        switch (curtype) {
@@ -855,7 +872,7 @@
 
  abort:
        (void)xsignal(SIGINT, oldintr);
-       oldintr = NULL;
+       oldintr = SIG_ERR;
        if (!cpend) {
                code = -1;
                goto cleanupsend;
@@ -874,10 +891,10 @@
                ptransfer(0);
 
  cleanupsend:
-       if (oldintr)
+       if (oldintr != SIG_ERR)
                (void)xsignal(SIGINT, oldintr);
-       if (oldintp)
-               (void)xsignal(SIGPIPE, oldintp);
+       if (oldpipe != SIG_ERR)
+               (void)xsignal(SIGPIPE, oldpipe);
        if (data >= 0) {
                (void)close(data);
                data = -1;
@@ -899,7 +916,7 @@
        FILE *volatile din;
        int (*volatile closefunc)(FILE *);
        sigfunc volatile oldintr;
-       sigfunc volatile oldintp;
+       sigfunc volatile oldpipe;
        int c, d;
        int volatile is_retr;
        int volatile tcrflag;
@@ -935,8 +952,8 @@
                return;
        }
        closefunc = NULL;
-       oldintr = NULL;
-       oldintp = NULL;
+       oldintr = SIG_ERR;
+       oldpipe = SIG_ERR;
        tcrflag = !crflag && is_retr;
        if (sigsetjmp(xferabort, 1)) {
                while (cpend)
@@ -1017,7 +1034,7 @@
                progress = 0;
                preserve = 0;
        } else if (!ignorespecial && *local == '|') {
-               oldintp = xsignal(SIGPIPE, SIG_IGN);
+               oldpipe = xsignal(SIGPIPE, SIG_IGN);
                fout = popen(local + 1, "w");
                if (fout == NULL) {
                        warn("Can't execute `%s'", local+1);
@@ -1183,10 +1200,10 @@
                ptransfer(0);
 
  cleanuprecv:
-       if (oldintr)
+       if (oldintr != SIG_ERR)
                (void)xsignal(SIGINT, oldintr);
-       if (oldintp)
-               (void)xsignal(SIGPIPE, oldintp);
+       if (oldpipe != SIG_ERR)
+               (void)xsignal(SIGPIPE, oldpipe);
        if (data >= 0) {
                (void)close(data);
                data = -1;
@@ -1856,7 +1873,7 @@
        int volatile secndflag;
        const char *volatile cmd2;
 
-       oldintr = NULL;
+       oldintr = SIG_ERR;
        secndflag = 0;
        if (strcmp(cmd, "RETR"))
                cmd2 = "RETR";
diff -r 4a59559a0b2c -r cbeab69e2641 usr.bin/ftp/version.h
--- a/usr.bin/ftp/version.h     Sat Jul 11 00:39:53 2020 +0000
+++ b/usr.bin/ftp/version.h     Sat Jul 11 02:19:31 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: version.h,v 1.89 2020/06/08 01:33:27 lukem Exp $       */
+/*     $NetBSD: version.h,v 1.90 2020/07/11 02:19:31 lukem Exp $       */
 
 /*-
  * Copyright (c) 1999-2020 The NetBSD Foundation, Inc.
@@ -34,5 +34,5 @@
 #endif
 
 #ifndef FTP_VERSION
-#define        FTP_VERSION     "20200608"
+#define        FTP_VERSION     "20200711"
 #endif